| Message ID | f89ec29e2087fcb0353e8d7634bdad8e5c0522c1.1401200582.git.jcody@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/27/2014 08:28 AM, Jeff Cody wrote: > Now that node-names can reference an individual BlockDriverState without > needing to navigate downwards from the 'device' level, in order to find > the top-most image (active layer) we need a pointer to the overlay of a > BDS. > > This will allow QMP commands to reference an image solely by its > node-name, without also needing to pass in the corresponding 'device' > string. > > Signed-off-by: Jeff Cody <jcody@redhat.com> > --- > block.c | 5 +++++ > include/block/block_int.h | 1 + > 2 files changed, 6 insertions(+) > Deceptively simple - good thing we already refactored all chain manipulations to go through bdrv_set_backing_hd(). Reviewed-by: Eric Blake <eblake@redhat.com>
On Tue, May 27, 2014 at 09:57:19AM -0600, Eric Blake wrote: > On 05/27/2014 08:28 AM, Jeff Cody wrote: > > Now that node-names can reference an individual BlockDriverState without > > needing to navigate downwards from the 'device' level, in order to find > > the top-most image (active layer) we need a pointer to the overlay of a > > BDS. > > > > This will allow QMP commands to reference an image solely by its > > node-name, without also needing to pass in the corresponding 'device' > > string. > > > > Signed-off-by: Jeff Cody <jcody@redhat.com> > > --- > > block.c | 5 +++++ > > include/block/block_int.h | 1 + > > 2 files changed, 6 insertions(+) > > > > Deceptively simple - good thing we already refactored all chain > manipulations to go through bdrv_set_backing_hd(). > Alas, it was too deceptive... it breaks in the bdrv_append() case. An example: Existing chain: [0 'base'] <--- [1 'sn1'] <--- [2 'sn2'] Perform a snapshot: [0 'base'] <--- [1 'sn1'] <--- [3 'sn2'] <---- [2 'sn3'] ^^^^^^^^^ (new BDS) [1] will have an overlay pointing to [2] still, skipping the newly injected BDS. When we perform bdrv_append(), we also need to update the overlay of the active layer's backing_hd BDS. Fortunately, this is just one additional line (although I am also adding a helper, since we just got rid of open coding bs->backing_hd, we should probably avoid doing the same thing with bs->overlay)
diff --git a/block.c b/block.c index cf4b296..b72fe4e 100644 --- a/block.c +++ b/block.c @@ -1114,6 +1114,9 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd) if (bs->backing_hd) { assert(bs->backing_blocker); bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker); + if (!backing_hd) { + bs->backing_hd->overlay = NULL; + } } else if (backing_hd) { error_setg(&bs->backing_blocker, "device is used as backing hd of '%s'", @@ -1126,6 +1129,8 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd) bs->backing_blocker = NULL; goto out; } + backing_hd->overlay = bs; + bs->open_flags &= ~BDRV_O_NO_BACKING; pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->filename); pstrcpy(bs->backing_format, sizeof(bs->backing_format), diff --git a/include/block/block_int.h b/include/block/block_int.h index f2e753f..c0fe90b 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -303,6 +303,7 @@ struct BlockDriverState { char backing_format[16]; /* if non-zero and backing_file exists */ BlockDriverState *backing_hd; + BlockDriverState *overlay; BlockDriverState *file; NotifierList close_notifiers;
Now that node-names can reference an individual BlockDriverState without needing to navigate downwards from the 'device' level, in order to find the top-most image (active layer) we need a pointer to the overlay of a BDS. This will allow QMP commands to reference an image solely by its node-name, without also needing to pass in the corresponding 'device' string. Signed-off-by: Jeff Cody <jcody@redhat.com> --- block.c | 5 +++++ include/block/block_int.h | 1 + 2 files changed, 6 insertions(+)