| Message ID | 1444680042-13207-19-git-send-email-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
Am 12.10.2015 um 22:00 hat Max Reitz geschrieben: > This structure will store some of the state of the root BDS if the BDS > tree is removed, so that state can be restored once a new BDS tree is > inserted. > > Signed-off-by: Max Reitz <mreitz@redhat.com> Random thought, not directly related to this series: We currently don't have a way to create just a BlockBackend with no medium. If we were to add that, would we want that to be just like a missing -drive file=... option, or would it actually make sense to specify the BlockBackendRootState? If we want to do that, we might actually have found a reason why the 'options' key makes sense in blockdev-add. We could make it a union where you either describe a BlockBackendRootState or a BDS. Another related question is whether we want to separate out BB options (which would otherwise be shared between the BBRS and BDS variants) into their own dict. This dict could be optional and that would be the way to specify whether you want a BB on top or not. Candidates for this dict are id/rerror/werror. There are more fields that exist in both the BDS and BBRS variants, but don't really belong to the BB (i.e. they end up in the real BBRS and not in the BB, and are only valid as long as no BDS is attached). These would not be moved to the BB options dict. Any opinions? Kevin
On 19.10.2015 16:18, Kevin Wolf wrote: > Am 12.10.2015 um 22:00 hat Max Reitz geschrieben: >> This structure will store some of the state of the root BDS if the BDS >> tree is removed, so that state can be restored once a new BDS tree is >> inserted. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> > > Random thought, not directly related to this series: > > We currently don't have a way to create just a BlockBackend with no > medium. If we were to add that, would we want that to be just like a > missing -drive file=... option, or would it actually make sense to > specify the BlockBackendRootState? We don't? -drive if=none,id=foo works just fine for me. Issuing qemu-io foo "read 0 512" then prints "read failed: " + strerror(ENOMEDIUM) to stderr. > If we want to do that, we might actually have found a reason why the > 'options' key makes sense in blockdev-add. We could make it a union > where you either describe a BlockBackendRootState or a BDS. I really wouldn't want to use the BBRS for anything but legacy support (i.e. change inheriting the options of the last medium)... > Another related question is whether we want to separate out BB options > (which would otherwise be shared between the BBRS and BDS variants) into > their own dict. This dict could be optional and that would be the way to > specify whether you want a BB on top or not. Candidates for this dict > are id/rerror/werror. > > There are more fields that exist in both the BDS and BBRS variants, but > don't really belong to the BB (i.e. they end up in the real BBRS and not > in the BB, and are only valid as long as no BDS is attached). These > would not be moved to the BB options dict. > > Any opinions? Not on the latter part. Max
Am 19.10.2015 um 16:32 hat Max Reitz geschrieben: > On 19.10.2015 16:18, Kevin Wolf wrote: > > Am 12.10.2015 um 22:00 hat Max Reitz geschrieben: > >> This structure will store some of the state of the root BDS if the BDS > >> tree is removed, so that state can be restored once a new BDS tree is > >> inserted. > >> > >> Signed-off-by: Max Reitz <mreitz@redhat.com> > > > > Random thought, not directly related to this series: > > > > We currently don't have a way to create just a BlockBackend with no > > medium. If we were to add that, would we want that to be just like a > > missing -drive file=... option, or would it actually make sense to > > specify the BlockBackendRootState? > > We don't? -drive if=none,id=foo works just fine for me. Issuing qemu-io > foo "read 0 512" then prints "read failed: " + strerror(ENOMEDIUM) to > stderr. Sorry, I meant "no blockdev-add way", i.e. hotplug an empty BB in QMP. > > If we want to do that, we might actually have found a reason why the > > 'options' key makes sense in blockdev-add. We could make it a union > > where you either describe a BlockBackendRootState or a BDS. > > I really wouldn't want to use the BBRS for anything but legacy support > (i.e. change inheriting the options of the last medium)... Fair enough. Then I guess it was a random stupid thought. > > Another related question is whether we want to separate out BB options > > (which would otherwise be shared between the BBRS and BDS variants) into > > their own dict. This dict could be optional and that would be the way to > > specify whether you want a BB on top or not. Candidates for this dict > > are id/rerror/werror. > > > > There are more fields that exist in both the BDS and BBRS variants, but > > don't really belong to the BB (i.e. they end up in the real BBRS and not > > in the BB, and are only valid as long as no BDS is attached). These > > would not be moved to the BB options dict. > > > > Any opinions? > > Not on the latter part. Pity. Kevin
diff --git a/block/block-backend.c b/block/block-backend.c index 2708ad1..6a3f0c7 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -13,6 +13,7 @@ #include "sysemu/block-backend.h" #include "block/block_int.h" #include "block/blockjob.h" +#include "block/throttle-groups.h" #include "sysemu/blockdev.h" #include "sysemu/sysemu.h" #include "qapi-event.h" @@ -37,6 +38,10 @@ struct BlockBackend { /* the block size for which the guest device expects atomicity */ int guest_block_size; + /* If the BDS tree is removed, some of its options are stored here (which + * can be used to restore those options in the new BDS on insert) */ + BlockBackendRootState root_state; + /* I/O stats (display with "info blockstats"). */ BlockAcctStats stats; @@ -161,6 +166,10 @@ static void blk_delete(BlockBackend *blk) bdrv_unref(blk->bs); blk->bs = NULL; } + if (blk->root_state.throttle_state) { + g_free(blk->root_state.throttle_group); + throttle_group_unref(blk->root_state.throttle_state); + } /* Avoid double-remove after blk_hide_on_behalf_of_hmp_drive_del() */ if (blk->name[0]) { QTAILQ_REMOVE(&blk_backends, blk, link); @@ -1067,3 +1076,34 @@ int blk_probe_geometry(BlockBackend *blk, HDGeometry *geo) { return bdrv_probe_geometry(blk->bs, geo); } + +/* + * Updates the BlockBackendRootState object with data from the currently + * attached BlockDriverState. + */ +void blk_update_root_state(BlockBackend *blk) +{ + assert(blk->bs); + + blk->root_state.open_flags = blk->bs->open_flags; + blk->root_state.read_only = blk->bs->read_only; + blk->root_state.detect_zeroes = blk->bs->detect_zeroes; + + if (blk->root_state.throttle_group) { + g_free(blk->root_state.throttle_group); + throttle_group_unref(blk->root_state.throttle_state); + } + if (blk->bs->throttle_state) { + const char *name = throttle_group_get_name(blk->bs); + blk->root_state.throttle_group = g_strdup(name); + blk->root_state.throttle_state = throttle_group_incref(name); + } else { + blk->root_state.throttle_group = NULL; + blk->root_state.throttle_state = NULL; + } +} + +BlockBackendRootState *blk_get_root_state(BlockBackend *blk) +{ + return &blk->root_state; +} diff --git a/include/block/block_int.h b/include/block/block_int.h index 009d6ea..e472a03 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -26,6 +26,7 @@ #include "block/accounting.h" #include "block/block.h" +#include "block/throttle-groups.h" #include "qemu/option.h" #include "qemu/queue.h" #include "block/coroutine.h" @@ -449,6 +450,15 @@ struct BlockDriverState { NotifierWithReturn write_threshold_notifier; }; +struct BlockBackendRootState { + int open_flags; + bool read_only; + BlockdevDetectZeroesOptions detect_zeroes; + + char *throttle_group; + ThrottleState *throttle_state; +}; + static inline BlockDriverState *backing_bs(BlockDriverState *bs) { return bs->backing ? bs->backing->bs : NULL; diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index ee1ce1d..2e39751 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -11,6 +11,7 @@ typedef struct AddressSpace AddressSpace; typedef struct AioContext AioContext; typedef struct AudioState AudioState; typedef struct BlockBackend BlockBackend; +typedef struct BlockBackendRootState BlockBackendRootState; typedef struct BlockDriverState BlockDriverState; typedef struct BusClass BusClass; typedef struct BusState BusState; diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index eafcef0..52e35a1 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -163,6 +163,8 @@ void blk_add_close_notifier(BlockBackend *blk, Notifier *notify); void blk_io_plug(BlockBackend *blk); void blk_io_unplug(BlockBackend *blk); BlockAcctStats *blk_get_stats(BlockBackend *blk); +BlockBackendRootState *blk_get_root_state(BlockBackend *blk); +void blk_update_root_state(BlockBackend *blk); void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk, BlockCompletionFunc *cb, void *opaque);
This structure will store some of the state of the root BDS if the BDS tree is removed, so that state can be restored once a new BDS tree is inserted. Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/block-backend.c | 40 ++++++++++++++++++++++++++++++++++++++++ include/block/block_int.h | 10 ++++++++++ include/qemu/typedefs.h | 1 + include/sysemu/block-backend.h | 2 ++ 4 files changed, 53 insertions(+)