Re: [Qemu-devel] [PATCH v7 31/39] blockdev: Add blockdev-insert-medium

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2864 bytes --]

On 21.10.2015 13:49, Alberto Garcia wrote:
> On Mon 19 Oct 2015 05:53:37 PM CEST, Max Reitz wrote:
>> And a helper function for that, which directly takes a pointer to the
>> BDS to be inserted instead of its node-name (which will be used for
>> implementing 'change' using blockdev-insert-medium).
> 
> Shouldn't this update bdrv_states?

I hate bdrv_states.

Yes, it should. Thanks!

Max

> Consider this scenario:
> 
> 1) We add a drive and eject its BDS
>    { "execute": "blockdev-add", "arguments": {
>          "options": { "driver": "qcow2",
>                       "file": { "driver": "file",
>                                 "filename": "/tmp/hd0.img"},
>                       "id": "drive0" }}}
> 
>    { "execute": "eject", "arguments": {"device": "drive0"}}
> 
> 2) We create a new BDS and insert it in drive0
>    { "execute": "blockdev-add", "arguments": {
>          "options": { "driver": "qcow2",
>                       "file": { "driver": "file",
>                                 "filename": "/tmp/hd0.img"},
>                       "node-name": "hd0" }}}
> 
>    { "execute": "blockdev-insert-medium", "arguments": {
>          "device": "drive0",
>          "node-name": "hd0" }}
> 
> 3) Now we try to create a snapshot...
> 
>    { "execute": "blockdev-snapshot-sync", "arguments": {
>          "device": "drive0",
>          "snapshot-file": "/tmp/new.img",
>          "format": "qcow2" }}
> 
>    {"error": {"class": "GenericError",
>               "desc": "The feature 'snapshot' is not enabled"}}
> 
>    Ooops! It seems that this is because the new node hd0 is not in the
>    bdrv_states list.
> 
> 4) Let's try to create a mirror instead
> 
>    { "execute": "drive-mirror", "arguments": {
>          "device": "drive0",
>          "target": "/tmp/new.img",
>          "sync": "top"}}
> 
>    {"return": {}}
>    {"timestamp": {"seconds": 1445427560,
>                   "microseconds": 765993},
>     "event": "BLOCK_JOB_READY",
>     "data": {"device": "drive0",
>              "len": 0,
>              "offset": 0,
>              "speed": 0,
>              "type": "mirror"}}
> 
> 5) Ok, the block job is ready, so let's complete it:
> 
>    { "execute": "query-block-jobs" }
>    {"return": []}
> 
>    Ooops! Again, hd0 is not in bdrv_states so QEMU cannot find the block
>    job.
> 
> 6) Anyway, we only need the backend name in order to complete a block
>    job, so surely we can do it even if it's not in the list:
> 
>    { "execute": "block-job-complete", "arguments": {
>          "device": "drive0"}}
> 
>    Segmentation fault
> 
>    That's QTAILQ_INSERT_BEFORE() in change_parent_backing_link(). This
>    code assumes that since the 'from' BDS is attached to a backend, it
>    must also be in bdrv_states.
> 
> Berto
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]