Re: [Qemu-devel] [PATCH v6 14/15] block: Rewrite bdrv_close_all()

[Max Reitz]

On 05.11.2015 18:15, Paolo Bonzini wrote:
> 
> 
> On 04/11/2015 19:57, Max Reitz wrote:
>>      BdrvAioNotifier *ban, *ban_next;
>>  
>> -    if (bs->job) {
>> -        block_job_cancel_sync(bs->job);
>> -    }
>> +    assert(!bs->job);
>>  
> 
> Who does this when ejecting a BDS from a BB?

Nobody, which one can see as good or bad, depending on whether you liked
it or not (it's mostly bad, though).

So by removing this without replacement, block jobs continue to work on
their BDS (and own it) until they're completed.

One stance you can take is: Block jobs work on the BDS, so ejecting the
BDS from the BB should not have an impact anyway (the BB name is only
used as a shorthand for identifying the BDS).

Another stance you can take is: The user only sees that the block job
takes a device name, so it looks like it's working on the BB level and
should behave as such.

The first point becomes moot when one realizes that you can no longer
complete a running block job once its BDS has been detached from the BB,
since the block job commands take BB names.

However, let's just try it:

$ x86_64-softmmu/qemu-system-x86_64 -qmp stdio \
  -drive if=none,file=test.qcow2,id=drive0,node-name=node0
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 4, "major": 2},
"package": ""}, "capabilities": []}}
{'execute':'qmp_capabilities'}
{"return": {}}
{'execute':'drive-backup','arguments':{'device':'drive0','target':'out.qcow2','format':'qcow2','sync':'none'}}
Formatting 'out.qcow2', fmt=qcow2 size=67108864 backing_file=test.qcow2
backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off
refcount_bits=16
{"return": {}}
{'execute':'eject','arguments':{'device':'drive0'}}
{"error": {"class": "GenericError", "desc": "Node 'drive0' is busy:
block device is in use by block job: backup"}}

So... Nobody does it, but apparently nobody has to because you cannot
eject (to be more specific: use blockdev-remove-medium) a BDS from a BB
while a block job is running on the BDS.

Max

signature.asc
Description: OpenPGP digital signature