Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium
On 11.09.2015 09:30, Wen Congyang wrote: > On 09/11/2015 03:09 AM, Max Reitz wrote: >> On 10.09.2015 03:12, Wen Congyang wrote: >>> On 09/09/2015 08:59 PM, Max Reitz wrote: On 09.09.2015 12:01, Wen Congyang wrote: > On 09/09/2015 05:20 AM, Max Reitz wrote: >> On 08.09.2015 11:13, Wen Congyang wrote: >>> On 07/21/2015 01:45 AM, 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). Signed-off-by: Max Reitz--- blockdev.c | 48 qapi/block-core.json | 17 + qmp-commands.hx | 37 + 3 files changed, 102 insertions(+) diff --git a/blockdev.c b/blockdev.c index 481760a..a80d0e2 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2164,6 +2164,54 @@ void qmp_blockdev_remove_medium(const char *device, Error **errp) } } +static void qmp_blockdev_insert_anon_medium(const char *device, +BlockDriverState *bs, Error **errp) +{ +BlockBackend *blk; +bool has_device; + +blk = blk_by_name(device); +if (!blk) { +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, + "Device '%s' not found", device); +return; +} + +/* For BBs without a device, we can exchange the BDS tree at will */ +has_device = blk_get_attached_dev(blk); + +if (has_device && !blk_dev_has_removable_media(blk)) { +error_setg(errp, "Device '%s' is not removable", device); +return; +} + +if (has_device && !blk_dev_is_tray_open(blk)) { +error_setg(errp, "Tray of device '%s' is not open", device); +return; +} + +if (blk_bs(blk)) { +error_setg(errp, "There already is a medium in device '%s'", device); +return; +} + +blk_insert_bs(blk, bs); +} + +void qmp_blockdev_insert_medium(const char *device, const char *node_name, +Error **errp) +{ +BlockDriverState *bs; + +bs = bdrv_find_node(node_name); +if (!bs) { +error_setg(errp, "Node '%s' not found", node_name); +return; +} >>> >>> Hmm, it is OK if the bs is not top BDS? >> >> I think so, yes. Generally, there's probably no reason to do that, but I >> don't know why we should not allow that case. For instance, you might >> want to make a backing file available read-only somewhere. >> >> It should be impossible to make it available writable, and it should not >> be allowed to start a block-commit operation while the backing file can >> be accessed by the guest, but this should be achieved using op blockers. >> >> What we need for this to work are fine-grained op blockers, I think. But >> working around that for now by only allowing to insert top BDS won't >> work, since you can still start block jobs which target top BDS, too >> (e.g. blockdev-backup can write to a BDS/BB that is visible to the >> guest). >> >> All in all, I think it's fine to insert non-top BDS, but we should >> definitely worry about which exact BDS one can insert once we have >> fine-grained op blockers. > > A BDS can be written by its parent, its block backend, a block job.. > So I think we should have some way to avoid more than two sources writing > to it, otherwise the data may be corrupted. Yes, and that would be op blockers. As I said, using blockdev-backup you can write to a BB that can be written to by the guest as well. I think this is a bug, but it is a bug that needs to be fixed by having better op blockers in place, which Jeff Cody is working on. Regarding this series, I don't consider this to be too big of an issue. Yes, if you are working with floppy disks, you can have the case of a block job and the guest writing to the BDS at the same time. But I can't really imagine who would use floppy disks and block jobs at the same time (people who still use floppy disks for their VMs don't strike me as the kind of people who use the management features of qemu, especially not for those floppy disks). Other than
Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium
On 09/11/2015 03:09 AM, Max Reitz wrote: > On 10.09.2015 03:12, Wen Congyang wrote: >> On 09/09/2015 08:59 PM, Max Reitz wrote: >>> On 09.09.2015 12:01, Wen Congyang wrote: On 09/09/2015 05:20 AM, Max Reitz wrote: > On 08.09.2015 11:13, Wen Congyang wrote: >> On 07/21/2015 01:45 AM, 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). >>> >>> Signed-off-by: Max Reitz>>> --- >>> blockdev.c | 48 >>> >>> qapi/block-core.json | 17 + >>> qmp-commands.hx | 37 + >>> 3 files changed, 102 insertions(+) >>> >>> diff --git a/blockdev.c b/blockdev.c >>> index 481760a..a80d0e2 100644 >>> --- a/blockdev.c >>> +++ b/blockdev.c >>> @@ -2164,6 +2164,54 @@ void qmp_blockdev_remove_medium(const char >>> *device, Error **errp) >>> } >>> } >>> >>> +static void qmp_blockdev_insert_anon_medium(const char *device, >>> +BlockDriverState *bs, >>> Error **errp) >>> +{ >>> +BlockBackend *blk; >>> +bool has_device; >>> + >>> +blk = blk_by_name(device); >>> +if (!blk) { >>> +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, >>> + "Device '%s' not found", device); >>> +return; >>> +} >>> + >>> +/* For BBs without a device, we can exchange the BDS tree at will >>> */ >>> +has_device = blk_get_attached_dev(blk); >>> + >>> +if (has_device && !blk_dev_has_removable_media(blk)) { >>> +error_setg(errp, "Device '%s' is not removable", device); >>> +return; >>> +} >>> + >>> +if (has_device && !blk_dev_is_tray_open(blk)) { >>> +error_setg(errp, "Tray of device '%s' is not open", device); >>> +return; >>> +} >>> + >>> +if (blk_bs(blk)) { >>> +error_setg(errp, "There already is a medium in device '%s'", >>> device); >>> +return; >>> +} >>> + >>> +blk_insert_bs(blk, bs); >>> +} >>> + >>> +void qmp_blockdev_insert_medium(const char *device, const char >>> *node_name, >>> +Error **errp) >>> +{ >>> +BlockDriverState *bs; >>> + >>> +bs = bdrv_find_node(node_name); >>> +if (!bs) { >>> +error_setg(errp, "Node '%s' not found", node_name); >>> +return; >>> +} >> >> Hmm, it is OK if the bs is not top BDS? > > I think so, yes. Generally, there's probably no reason to do that, but I > don't know why we should not allow that case. For instance, you might > want to make a backing file available read-only somewhere. > > It should be impossible to make it available writable, and it should not > be allowed to start a block-commit operation while the backing file can > be accessed by the guest, but this should be achieved using op blockers. > > What we need for this to work are fine-grained op blockers, I think. But > working around that for now by only allowing to insert top BDS won't > work, since you can still start block jobs which target top BDS, too > (e.g. blockdev-backup can write to a BDS/BB that is visible to the guest). > > All in all, I think it's fine to insert non-top BDS, but we should > definitely worry about which exact BDS one can insert once we have > fine-grained op blockers. A BDS can be written by its parent, its block backend, a block job.. So I think we should have some way to avoid more than two sources writing to it, otherwise the data may be corrupted. >>> >>> Yes, and that would be op blockers. >>> >>> As I said, using blockdev-backup you can write to a BB that can be >>> written to by the guest as well. I think this is a bug, but it is a bug >>> that needs to be fixed by having better op blockers in place, which Jeff >>> Cody is working on. >>> >>> Regarding this series, I don't consider this to be too big of an issue. >>> Yes, if you are working with floppy disks, you can have the case of a >>> block job and the guest writing to the BDS at the same time. But I can't >>> really imagine who would use floppy disks and block jobs at the same >>> time (people who still use floppy disks for their VMs don't strike me as >>> the kind of people who use the management features of qemu, especially >>> not for those floppy disks). >>> >>> Other than that, this function (blockdev-insert-medium) can only be used >>> for optical ROM devices (I don't think we have CD/DVD-RW support, do >>> we?), so it's much less of an
Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium
On 10.09.2015 03:12, Wen Congyang wrote: > On 09/09/2015 08:59 PM, Max Reitz wrote: >> On 09.09.2015 12:01, Wen Congyang wrote: >>> On 09/09/2015 05:20 AM, Max Reitz wrote: On 08.09.2015 11:13, Wen Congyang wrote: > On 07/21/2015 01:45 AM, 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). >> >> Signed-off-by: Max Reitz>> --- >> blockdev.c | 48 >> >> qapi/block-core.json | 17 + >> qmp-commands.hx | 37 + >> 3 files changed, 102 insertions(+) >> >> diff --git a/blockdev.c b/blockdev.c >> index 481760a..a80d0e2 100644 >> --- a/blockdev.c >> +++ b/blockdev.c >> @@ -2164,6 +2164,54 @@ void qmp_blockdev_remove_medium(const char >> *device, Error **errp) >> } >> } >> >> +static void qmp_blockdev_insert_anon_medium(const char *device, >> +BlockDriverState *bs, Error >> **errp) >> +{ >> +BlockBackend *blk; >> +bool has_device; >> + >> +blk = blk_by_name(device); >> +if (!blk) { >> +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, >> + "Device '%s' not found", device); >> +return; >> +} >> + >> +/* For BBs without a device, we can exchange the BDS tree at will */ >> +has_device = blk_get_attached_dev(blk); >> + >> +if (has_device && !blk_dev_has_removable_media(blk)) { >> +error_setg(errp, "Device '%s' is not removable", device); >> +return; >> +} >> + >> +if (has_device && !blk_dev_is_tray_open(blk)) { >> +error_setg(errp, "Tray of device '%s' is not open", device); >> +return; >> +} >> + >> +if (blk_bs(blk)) { >> +error_setg(errp, "There already is a medium in device '%s'", >> device); >> +return; >> +} >> + >> +blk_insert_bs(blk, bs); >> +} >> + >> +void qmp_blockdev_insert_medium(const char *device, const char >> *node_name, >> +Error **errp) >> +{ >> +BlockDriverState *bs; >> + >> +bs = bdrv_find_node(node_name); >> +if (!bs) { >> +error_setg(errp, "Node '%s' not found", node_name); >> +return; >> +} > > Hmm, it is OK if the bs is not top BDS? I think so, yes. Generally, there's probably no reason to do that, but I don't know why we should not allow that case. For instance, you might want to make a backing file available read-only somewhere. It should be impossible to make it available writable, and it should not be allowed to start a block-commit operation while the backing file can be accessed by the guest, but this should be achieved using op blockers. What we need for this to work are fine-grained op blockers, I think. But working around that for now by only allowing to insert top BDS won't work, since you can still start block jobs which target top BDS, too (e.g. blockdev-backup can write to a BDS/BB that is visible to the guest). All in all, I think it's fine to insert non-top BDS, but we should definitely worry about which exact BDS one can insert once we have fine-grained op blockers. >>> >>> A BDS can be written by its parent, its block backend, a block job.. >>> So I think we should have some way to avoid more than two sources writing >>> to it, otherwise the data may be corrupted. >> >> Yes, and that would be op blockers. >> >> As I said, using blockdev-backup you can write to a BB that can be >> written to by the guest as well. I think this is a bug, but it is a bug >> that needs to be fixed by having better op blockers in place, which Jeff >> Cody is working on. >> >> Regarding this series, I don't consider this to be too big of an issue. >> Yes, if you are working with floppy disks, you can have the case of a >> block job and the guest writing to the BDS at the same time. But I can't >> really imagine who would use floppy disks and block jobs at the same >> time (people who still use floppy disks for their VMs don't strike me as >> the kind of people who use the management features of qemu, especially >> not for those floppy disks). >> >> Other than that, this function (blockdev-insert-medium) can only be used >> for optical ROM devices (I don't think we have CD/DVD-RW support, do >> we?), so it's much less of an issue there. >> >> So all in all I don't consider this too big of an issue here. If others >> think different, then I would delay this part of the series (which >>
Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium
On 10.09.2015 05:22, Wen Congyang wrote: > On 09/09/2015 08:59 PM, Max Reitz wrote: >> On 09.09.2015 12:01, Wen Congyang wrote: >>> On 09/09/2015 05:20 AM, Max Reitz wrote: On 08.09.2015 11:13, Wen Congyang wrote: > On 07/21/2015 01:45 AM, 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). >> >> Signed-off-by: Max Reitz>> --- >> blockdev.c | 48 >> >> qapi/block-core.json | 17 + >> qmp-commands.hx | 37 + >> 3 files changed, 102 insertions(+) >> >> diff --git a/blockdev.c b/blockdev.c >> index 481760a..a80d0e2 100644 >> --- a/blockdev.c >> +++ b/blockdev.c >> @@ -2164,6 +2164,54 @@ void qmp_blockdev_remove_medium(const char >> *device, Error **errp) >> } >> } >> >> +static void qmp_blockdev_insert_anon_medium(const char *device, >> +BlockDriverState *bs, Error >> **errp) >> +{ >> +BlockBackend *blk; >> +bool has_device; >> + >> +blk = blk_by_name(device); >> +if (!blk) { >> +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, >> + "Device '%s' not found", device); >> +return; >> +} >> + >> +/* For BBs without a device, we can exchange the BDS tree at will */ >> +has_device = blk_get_attached_dev(blk); >> + >> +if (has_device && !blk_dev_has_removable_media(blk)) { >> +error_setg(errp, "Device '%s' is not removable", device); >> +return; >> +} >> + >> +if (has_device && !blk_dev_is_tray_open(blk)) { >> +error_setg(errp, "Tray of device '%s' is not open", device); >> +return; >> +} >> + >> +if (blk_bs(blk)) { >> +error_setg(errp, "There already is a medium in device '%s'", >> device); >> +return; >> +} >> + >> +blk_insert_bs(blk, bs); >> +} >> + >> +void qmp_blockdev_insert_medium(const char *device, const char >> *node_name, >> +Error **errp) >> +{ >> +BlockDriverState *bs; >> + >> +bs = bdrv_find_node(node_name); >> +if (!bs) { >> +error_setg(errp, "Node '%s' not found", node_name); >> +return; >> +} > > Hmm, it is OK if the bs is not top BDS? I think so, yes. Generally, there's probably no reason to do that, but I don't know why we should not allow that case. For instance, you might want to make a backing file available read-only somewhere. It should be impossible to make it available writable, and it should not be allowed to start a block-commit operation while the backing file can be accessed by the guest, but this should be achieved using op blockers. What we need for this to work are fine-grained op blockers, I think. But working around that for now by only allowing to insert top BDS won't work, since you can still start block jobs which target top BDS, too (e.g. blockdev-backup can write to a BDS/BB that is visible to the guest). All in all, I think it's fine to insert non-top BDS, but we should definitely worry about which exact BDS one can insert once we have fine-grained op blockers. >>> >>> A BDS can be written by its parent, its block backend, a block job.. >>> So I think we should have some way to avoid more than two sources writing >>> to it, otherwise the data may be corrupted. >> >> Yes, and that would be op blockers. >> >> As I said, using blockdev-backup you can write to a BB that can be >> written to by the guest as well. I think this is a bug, but it is a bug >> that needs to be fixed by having better op blockers in place, which Jeff >> Cody is working on. > > I don't find such patches in the maillist. That's because Jeff is still working on designing and writing them. Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium
On 09/09/2015 05:20 AM, Max Reitz wrote: > On 08.09.2015 11:13, Wen Congyang wrote: >> On 07/21/2015 01:45 AM, 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). >>> >>> Signed-off-by: Max Reitz>>> --- >>> blockdev.c | 48 >>> qapi/block-core.json | 17 + >>> qmp-commands.hx | 37 + >>> 3 files changed, 102 insertions(+) >>> >>> diff --git a/blockdev.c b/blockdev.c >>> index 481760a..a80d0e2 100644 >>> --- a/blockdev.c >>> +++ b/blockdev.c >>> @@ -2164,6 +2164,54 @@ void qmp_blockdev_remove_medium(const char *device, >>> Error **errp) >>> } >>> } >>> >>> +static void qmp_blockdev_insert_anon_medium(const char *device, >>> +BlockDriverState *bs, Error >>> **errp) >>> +{ >>> +BlockBackend *blk; >>> +bool has_device; >>> + >>> +blk = blk_by_name(device); >>> +if (!blk) { >>> +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, >>> + "Device '%s' not found", device); >>> +return; >>> +} >>> + >>> +/* For BBs without a device, we can exchange the BDS tree at will */ >>> +has_device = blk_get_attached_dev(blk); >>> + >>> +if (has_device && !blk_dev_has_removable_media(blk)) { >>> +error_setg(errp, "Device '%s' is not removable", device); >>> +return; >>> +} >>> + >>> +if (has_device && !blk_dev_is_tray_open(blk)) { >>> +error_setg(errp, "Tray of device '%s' is not open", device); >>> +return; >>> +} >>> + >>> +if (blk_bs(blk)) { >>> +error_setg(errp, "There already is a medium in device '%s'", >>> device); >>> +return; >>> +} >>> + >>> +blk_insert_bs(blk, bs); >>> +} >>> + >>> +void qmp_blockdev_insert_medium(const char *device, const char *node_name, >>> +Error **errp) >>> +{ >>> +BlockDriverState *bs; >>> + >>> +bs = bdrv_find_node(node_name); >>> +if (!bs) { >>> +error_setg(errp, "Node '%s' not found", node_name); >>> +return; >>> +} >> >> Hmm, it is OK if the bs is not top BDS? > > I think so, yes. Generally, there's probably no reason to do that, but I > don't know why we should not allow that case. For instance, you might > want to make a backing file available read-only somewhere. > > It should be impossible to make it available writable, and it should not > be allowed to start a block-commit operation while the backing file can > be accessed by the guest, but this should be achieved using op blockers. > > What we need for this to work are fine-grained op blockers, I think. But > working around that for now by only allowing to insert top BDS won't > work, since you can still start block jobs which target top BDS, too > (e.g. blockdev-backup can write to a BDS/BB that is visible to the guest). > > All in all, I think it's fine to insert non-top BDS, but we should > definitely worry about which exact BDS one can insert once we have > fine-grained op blockers. A BDS can be written by its parent, its block backend, a block job.. So I think we should have some way to avoid more than two sources writing to it, otherwise the data may be corrupted. Thanks Wen Congyang > > Max > >> Thanks >> Wen Congyang >> >>> + >>> +qmp_blockdev_insert_anon_medium(device, bs, errp); >>> +} >>> + >>> /* throttling disk I/O limits */ >>> void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t >>> bps_rd, >>> int64_t bps_wr, >>> diff --git a/qapi/block-core.json b/qapi/block-core.json >>> index 63a83e4..84c9b23 100644 >>> --- a/qapi/block-core.json >>> +++ b/qapi/block-core.json >>> @@ -1925,6 +1925,23 @@ >>> { 'command': 'blockdev-remove-medium', >>>'data': { 'device': 'str' } } >>> >>> +## >>> +# @blockdev-insert-medium: >>> +# >>> +# Inserts a medium (a block driver state tree) into a block device. That >>> block >>> +# device's tray must currently be open and there must be no medium inserted >>> +# already. >>> +# >>> +# @device:block device name >>> +# >>> +# @node-name: name of a node in the block driver state graph >>> +# >>> +# Since: 2.5 >>> +## >>> +{ 'command': 'blockdev-insert-medium', >>> + 'data': { 'device': 'str', >>> +'node-name': 'str'} } >>> + >>> >>> ## >>> # @BlockErrorAction >>> diff --git a/qmp-commands.hx b/qmp-commands.hx >>> index ff6c572..b4c34fe 100644 >>> --- a/qmp-commands.hx >>> +++ b/qmp-commands.hx >>> @@ -3991,6 +3991,43 @@ Example: >>> EQMP >>> >>> { >>> +.name = "blockdev-insert-medium", >>> +.args_type = "device:s,node-name:s", >>> +.mhandler.cmd_new = qmp_marshal_input_blockdev_insert_medium, >>> +}, >>> + >>>
Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium
On 09.09.2015 12:01, Wen Congyang wrote: > On 09/09/2015 05:20 AM, Max Reitz wrote: >> On 08.09.2015 11:13, Wen Congyang wrote: >>> On 07/21/2015 01:45 AM, 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). Signed-off-by: Max Reitz--- blockdev.c | 48 qapi/block-core.json | 17 + qmp-commands.hx | 37 + 3 files changed, 102 insertions(+) diff --git a/blockdev.c b/blockdev.c index 481760a..a80d0e2 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2164,6 +2164,54 @@ void qmp_blockdev_remove_medium(const char *device, Error **errp) } } +static void qmp_blockdev_insert_anon_medium(const char *device, +BlockDriverState *bs, Error **errp) +{ +BlockBackend *blk; +bool has_device; + +blk = blk_by_name(device); +if (!blk) { +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, + "Device '%s' not found", device); +return; +} + +/* For BBs without a device, we can exchange the BDS tree at will */ +has_device = blk_get_attached_dev(blk); + +if (has_device && !blk_dev_has_removable_media(blk)) { +error_setg(errp, "Device '%s' is not removable", device); +return; +} + +if (has_device && !blk_dev_is_tray_open(blk)) { +error_setg(errp, "Tray of device '%s' is not open", device); +return; +} + +if (blk_bs(blk)) { +error_setg(errp, "There already is a medium in device '%s'", device); +return; +} + +blk_insert_bs(blk, bs); +} + +void qmp_blockdev_insert_medium(const char *device, const char *node_name, +Error **errp) +{ +BlockDriverState *bs; + +bs = bdrv_find_node(node_name); +if (!bs) { +error_setg(errp, "Node '%s' not found", node_name); +return; +} >>> >>> Hmm, it is OK if the bs is not top BDS? >> >> I think so, yes. Generally, there's probably no reason to do that, but I >> don't know why we should not allow that case. For instance, you might >> want to make a backing file available read-only somewhere. >> >> It should be impossible to make it available writable, and it should not >> be allowed to start a block-commit operation while the backing file can >> be accessed by the guest, but this should be achieved using op blockers. >> >> What we need for this to work are fine-grained op blockers, I think. But >> working around that for now by only allowing to insert top BDS won't >> work, since you can still start block jobs which target top BDS, too >> (e.g. blockdev-backup can write to a BDS/BB that is visible to the guest). >> >> All in all, I think it's fine to insert non-top BDS, but we should >> definitely worry about which exact BDS one can insert once we have >> fine-grained op blockers. > > A BDS can be written by its parent, its block backend, a block job.. > So I think we should have some way to avoid more than two sources writing > to it, otherwise the data may be corrupted. Yes, and that would be op blockers. As I said, using blockdev-backup you can write to a BB that can be written to by the guest as well. I think this is a bug, but it is a bug that needs to be fixed by having better op blockers in place, which Jeff Cody is working on. Regarding this series, I don't consider this to be too big of an issue. Yes, if you are working with floppy disks, you can have the case of a block job and the guest writing to the BDS at the same time. But I can't really imagine who would use floppy disks and block jobs at the same time (people who still use floppy disks for their VMs don't strike me as the kind of people who use the management features of qemu, especially not for those floppy disks). Other than that, this function (blockdev-insert-medium) can only be used for optical ROM devices (I don't think we have CD/DVD-RW support, do we?), so it's much less of an issue there. So all in all I don't consider this too big of an issue here. If others think different, then I would delay this part of the series (which overhauls the "change" command) until we have fine-grained op blockers. Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium
On 09/09/2015 08:59 PM, Max Reitz wrote: > On 09.09.2015 12:01, Wen Congyang wrote: >> On 09/09/2015 05:20 AM, Max Reitz wrote: >>> On 08.09.2015 11:13, Wen Congyang wrote: On 07/21/2015 01:45 AM, 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). > > Signed-off-by: Max Reitz> --- > blockdev.c | 48 > > qapi/block-core.json | 17 + > qmp-commands.hx | 37 + > 3 files changed, 102 insertions(+) > > diff --git a/blockdev.c b/blockdev.c > index 481760a..a80d0e2 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -2164,6 +2164,54 @@ void qmp_blockdev_remove_medium(const char > *device, Error **errp) > } > } > > +static void qmp_blockdev_insert_anon_medium(const char *device, > +BlockDriverState *bs, Error > **errp) > +{ > +BlockBackend *blk; > +bool has_device; > + > +blk = blk_by_name(device); > +if (!blk) { > +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device); > +return; > +} > + > +/* For BBs without a device, we can exchange the BDS tree at will */ > +has_device = blk_get_attached_dev(blk); > + > +if (has_device && !blk_dev_has_removable_media(blk)) { > +error_setg(errp, "Device '%s' is not removable", device); > +return; > +} > + > +if (has_device && !blk_dev_is_tray_open(blk)) { > +error_setg(errp, "Tray of device '%s' is not open", device); > +return; > +} > + > +if (blk_bs(blk)) { > +error_setg(errp, "There already is a medium in device '%s'", > device); > +return; > +} > + > +blk_insert_bs(blk, bs); > +} > + > +void qmp_blockdev_insert_medium(const char *device, const char > *node_name, > +Error **errp) > +{ > +BlockDriverState *bs; > + > +bs = bdrv_find_node(node_name); > +if (!bs) { > +error_setg(errp, "Node '%s' not found", node_name); > +return; > +} Hmm, it is OK if the bs is not top BDS? >>> >>> I think so, yes. Generally, there's probably no reason to do that, but I >>> don't know why we should not allow that case. For instance, you might >>> want to make a backing file available read-only somewhere. >>> >>> It should be impossible to make it available writable, and it should not >>> be allowed to start a block-commit operation while the backing file can >>> be accessed by the guest, but this should be achieved using op blockers. >>> >>> What we need for this to work are fine-grained op blockers, I think. But >>> working around that for now by only allowing to insert top BDS won't >>> work, since you can still start block jobs which target top BDS, too >>> (e.g. blockdev-backup can write to a BDS/BB that is visible to the guest). >>> >>> All in all, I think it's fine to insert non-top BDS, but we should >>> definitely worry about which exact BDS one can insert once we have >>> fine-grained op blockers. >> >> A BDS can be written by its parent, its block backend, a block job.. >> So I think we should have some way to avoid more than two sources writing >> to it, otherwise the data may be corrupted. > > Yes, and that would be op blockers. > > As I said, using blockdev-backup you can write to a BB that can be > written to by the guest as well. I think this is a bug, but it is a bug > that needs to be fixed by having better op blockers in place, which Jeff > Cody is working on. > > Regarding this series, I don't consider this to be too big of an issue. > Yes, if you are working with floppy disks, you can have the case of a > block job and the guest writing to the BDS at the same time. But I can't > really imagine who would use floppy disks and block jobs at the same > time (people who still use floppy disks for their VMs don't strike me as > the kind of people who use the management features of qemu, especially > not for those floppy disks). > > Other than that, this function (blockdev-insert-medium) can only be used > for optical ROM devices (I don't think we have CD/DVD-RW support, do > we?), so it's much less of an issue there. > > So all in all I don't consider this too big of an issue here. If others > think different, then I would delay this part of the series (which > overhauls the "change" command) until we have fine-grained op blockers. In most cases, the user uses this command to change CD/DVD media, so it is OK. But IIRC
Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium
On 09/09/2015 08:59 PM, Max Reitz wrote: > On 09.09.2015 12:01, Wen Congyang wrote: >> On 09/09/2015 05:20 AM, Max Reitz wrote: >>> On 08.09.2015 11:13, Wen Congyang wrote: On 07/21/2015 01:45 AM, 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). > > Signed-off-by: Max Reitz> --- > blockdev.c | 48 > > qapi/block-core.json | 17 + > qmp-commands.hx | 37 + > 3 files changed, 102 insertions(+) > > diff --git a/blockdev.c b/blockdev.c > index 481760a..a80d0e2 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -2164,6 +2164,54 @@ void qmp_blockdev_remove_medium(const char > *device, Error **errp) > } > } > > +static void qmp_blockdev_insert_anon_medium(const char *device, > +BlockDriverState *bs, Error > **errp) > +{ > +BlockBackend *blk; > +bool has_device; > + > +blk = blk_by_name(device); > +if (!blk) { > +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device); > +return; > +} > + > +/* For BBs without a device, we can exchange the BDS tree at will */ > +has_device = blk_get_attached_dev(blk); > + > +if (has_device && !blk_dev_has_removable_media(blk)) { > +error_setg(errp, "Device '%s' is not removable", device); > +return; > +} > + > +if (has_device && !blk_dev_is_tray_open(blk)) { > +error_setg(errp, "Tray of device '%s' is not open", device); > +return; > +} > + > +if (blk_bs(blk)) { > +error_setg(errp, "There already is a medium in device '%s'", > device); > +return; > +} > + > +blk_insert_bs(blk, bs); > +} > + > +void qmp_blockdev_insert_medium(const char *device, const char > *node_name, > +Error **errp) > +{ > +BlockDriverState *bs; > + > +bs = bdrv_find_node(node_name); > +if (!bs) { > +error_setg(errp, "Node '%s' not found", node_name); > +return; > +} Hmm, it is OK if the bs is not top BDS? >>> >>> I think so, yes. Generally, there's probably no reason to do that, but I >>> don't know why we should not allow that case. For instance, you might >>> want to make a backing file available read-only somewhere. >>> >>> It should be impossible to make it available writable, and it should not >>> be allowed to start a block-commit operation while the backing file can >>> be accessed by the guest, but this should be achieved using op blockers. >>> >>> What we need for this to work are fine-grained op blockers, I think. But >>> working around that for now by only allowing to insert top BDS won't >>> work, since you can still start block jobs which target top BDS, too >>> (e.g. blockdev-backup can write to a BDS/BB that is visible to the guest). >>> >>> All in all, I think it's fine to insert non-top BDS, but we should >>> definitely worry about which exact BDS one can insert once we have >>> fine-grained op blockers. >> >> A BDS can be written by its parent, its block backend, a block job.. >> So I think we should have some way to avoid more than two sources writing >> to it, otherwise the data may be corrupted. > > Yes, and that would be op blockers. > > As I said, using blockdev-backup you can write to a BB that can be > written to by the guest as well. I think this is a bug, but it is a bug > that needs to be fixed by having better op blockers in place, which Jeff > Cody is working on. I don't find such patches in the maillist. Thanks Wen Congyang > > Regarding this series, I don't consider this to be too big of an issue. > Yes, if you are working with floppy disks, you can have the case of a > block job and the guest writing to the BDS at the same time. But I can't > really imagine who would use floppy disks and block jobs at the same > time (people who still use floppy disks for their VMs don't strike me as > the kind of people who use the management features of qemu, especially > not for those floppy disks). > > Other than that, this function (blockdev-insert-medium) can only be used > for optical ROM devices (I don't think we have CD/DVD-RW support, do > we?), so it's much less of an issue there. > > So all in all I don't consider this too big of an issue here. If others > think different, then I would delay this part of the series (which > overhauls the "change" command) until we have fine-grained op blockers. > > Max >
Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium
On 07/21/2015 01:45 AM, 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). > > Signed-off-by: Max Reitz> --- > blockdev.c | 48 > qapi/block-core.json | 17 + > qmp-commands.hx | 37 + > 3 files changed, 102 insertions(+) > > diff --git a/blockdev.c b/blockdev.c > index 481760a..a80d0e2 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -2164,6 +2164,54 @@ void qmp_blockdev_remove_medium(const char *device, > Error **errp) > } > } > > +static void qmp_blockdev_insert_anon_medium(const char *device, > +BlockDriverState *bs, Error > **errp) > +{ > +BlockBackend *blk; > +bool has_device; > + > +blk = blk_by_name(device); > +if (!blk) { > +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device); > +return; > +} > + > +/* For BBs without a device, we can exchange the BDS tree at will */ > +has_device = blk_get_attached_dev(blk); > + > +if (has_device && !blk_dev_has_removable_media(blk)) { > +error_setg(errp, "Device '%s' is not removable", device); > +return; > +} > + > +if (has_device && !blk_dev_is_tray_open(blk)) { > +error_setg(errp, "Tray of device '%s' is not open", device); > +return; > +} > + > +if (blk_bs(blk)) { > +error_setg(errp, "There already is a medium in device '%s'", device); > +return; > +} > + > +blk_insert_bs(blk, bs); > +} > + > +void qmp_blockdev_insert_medium(const char *device, const char *node_name, > +Error **errp) > +{ > +BlockDriverState *bs; > + > +bs = bdrv_find_node(node_name); > +if (!bs) { > +error_setg(errp, "Node '%s' not found", node_name); > +return; > +} Hmm, it is OK if the bs is not top BDS? Thanks Wen Congyang > + > +qmp_blockdev_insert_anon_medium(device, bs, errp); > +} > + > /* throttling disk I/O limits */ > void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t > bps_rd, > int64_t bps_wr, > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 63a83e4..84c9b23 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -1925,6 +1925,23 @@ > { 'command': 'blockdev-remove-medium', >'data': { 'device': 'str' } } > > +## > +# @blockdev-insert-medium: > +# > +# Inserts a medium (a block driver state tree) into a block device. That > block > +# device's tray must currently be open and there must be no medium inserted > +# already. > +# > +# @device:block device name > +# > +# @node-name: name of a node in the block driver state graph > +# > +# Since: 2.5 > +## > +{ 'command': 'blockdev-insert-medium', > + 'data': { 'device': 'str', > +'node-name': 'str'} } > + > > ## > # @BlockErrorAction > diff --git a/qmp-commands.hx b/qmp-commands.hx > index ff6c572..b4c34fe 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -3991,6 +3991,43 @@ Example: > EQMP > > { > +.name = "blockdev-insert-medium", > +.args_type = "device:s,node-name:s", > +.mhandler.cmd_new = qmp_marshal_input_blockdev_insert_medium, > +}, > + > +SQMP > +blockdev-insert-medium > +-- > + > +Inserts a medium (a block driver state tree) into a block device. That block > +device's tray must currently be open and there must be no medium inserted > +already. > + > +Arguments: > + > +- "device": block device name (json-string) > +- "node-name": root node of the BDS tree to insert into the block device > + > +Example: > + > +-> { "execute": "blockdev-add", > + "arguments": { "options": { "node-name": "node0", > + "driver": "raw", > + "file": { "driver": "file", > + "filename": "fedora.iso" } } } } > + > +<- { "return": {} } > + > +-> { "execute": "blockdev-insert-medium", > + "arguments": { "device": "ide1-cd0", > +"node-name": "node0" } } > + > +<- { "return": {} } > + > +EQMP > + > +{ > .name = "query-named-block-nodes", > .args_type = "", > .mhandler.cmd_new = qmp_marshal_input_query_named_block_nodes, >
Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium
On 08.09.2015 11:13, Wen Congyang wrote: > On 07/21/2015 01:45 AM, 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). >> >> Signed-off-by: Max Reitz>> --- >> blockdev.c | 48 >> qapi/block-core.json | 17 + >> qmp-commands.hx | 37 + >> 3 files changed, 102 insertions(+) >> >> diff --git a/blockdev.c b/blockdev.c >> index 481760a..a80d0e2 100644 >> --- a/blockdev.c >> +++ b/blockdev.c >> @@ -2164,6 +2164,54 @@ void qmp_blockdev_remove_medium(const char *device, >> Error **errp) >> } >> } >> >> +static void qmp_blockdev_insert_anon_medium(const char *device, >> +BlockDriverState *bs, Error >> **errp) >> +{ >> +BlockBackend *blk; >> +bool has_device; >> + >> +blk = blk_by_name(device); >> +if (!blk) { >> +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, >> + "Device '%s' not found", device); >> +return; >> +} >> + >> +/* For BBs without a device, we can exchange the BDS tree at will */ >> +has_device = blk_get_attached_dev(blk); >> + >> +if (has_device && !blk_dev_has_removable_media(blk)) { >> +error_setg(errp, "Device '%s' is not removable", device); >> +return; >> +} >> + >> +if (has_device && !blk_dev_is_tray_open(blk)) { >> +error_setg(errp, "Tray of device '%s' is not open", device); >> +return; >> +} >> + >> +if (blk_bs(blk)) { >> +error_setg(errp, "There already is a medium in device '%s'", >> device); >> +return; >> +} >> + >> +blk_insert_bs(blk, bs); >> +} >> + >> +void qmp_blockdev_insert_medium(const char *device, const char *node_name, >> +Error **errp) >> +{ >> +BlockDriverState *bs; >> + >> +bs = bdrv_find_node(node_name); >> +if (!bs) { >> +error_setg(errp, "Node '%s' not found", node_name); >> +return; >> +} > > Hmm, it is OK if the bs is not top BDS? I think so, yes. Generally, there's probably no reason to do that, but I don't know why we should not allow that case. For instance, you might want to make a backing file available read-only somewhere. It should be impossible to make it available writable, and it should not be allowed to start a block-commit operation while the backing file can be accessed by the guest, but this should be achieved using op blockers. What we need for this to work are fine-grained op blockers, I think. But working around that for now by only allowing to insert top BDS won't work, since you can still start block jobs which target top BDS, too (e.g. blockdev-backup can write to a BDS/BB that is visible to the guest). All in all, I think it's fine to insert non-top BDS, but we should definitely worry about which exact BDS one can insert once we have fine-grained op blockers. Max > Thanks > Wen Congyang > >> + >> +qmp_blockdev_insert_anon_medium(device, bs, errp); >> +} >> + >> /* throttling disk I/O limits */ >> void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t >> bps_rd, >> int64_t bps_wr, >> diff --git a/qapi/block-core.json b/qapi/block-core.json >> index 63a83e4..84c9b23 100644 >> --- a/qapi/block-core.json >> +++ b/qapi/block-core.json >> @@ -1925,6 +1925,23 @@ >> { 'command': 'blockdev-remove-medium', >>'data': { 'device': 'str' } } >> >> +## >> +# @blockdev-insert-medium: >> +# >> +# Inserts a medium (a block driver state tree) into a block device. That >> block >> +# device's tray must currently be open and there must be no medium inserted >> +# already. >> +# >> +# @device:block device name >> +# >> +# @node-name: name of a node in the block driver state graph >> +# >> +# Since: 2.5 >> +## >> +{ 'command': 'blockdev-insert-medium', >> + 'data': { 'device': 'str', >> +'node-name': 'str'} } >> + >> >> ## >> # @BlockErrorAction >> diff --git a/qmp-commands.hx b/qmp-commands.hx >> index ff6c572..b4c34fe 100644 >> --- a/qmp-commands.hx >> +++ b/qmp-commands.hx >> @@ -3991,6 +3991,43 @@ Example: >> EQMP >> >> { >> +.name = "blockdev-insert-medium", >> +.args_type = "device:s,node-name:s", >> +.mhandler.cmd_new = qmp_marshal_input_blockdev_insert_medium, >> +}, >> + >> +SQMP >> +blockdev-insert-medium >> +-- >> + >> +Inserts a medium (a block driver state tree) into a block device. That block >> +device's tray must currently be open and there must be no medium inserted >> +already. >> + >> +Arguments: >> + >> +- "device": block device name (json-string) >> +- "node-name": root node of the BDS tree to insert into the block device >> + >> +Example: >> + >> +->
Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium
On 09/07/2015 11:53 PM, Wen Congyang wrote: > On 07/21/2015 01:45 AM, 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). > > Is it OK to insert a medium to more than one BB? A read-only medium - sure :) A read-write medium - probably a recipe for data corruption. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium
On 07/21/2015 01:45 AM, 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). Is it OK to insert a medium to more than one BB? Thanks Wen Congyang > > Signed-off-by: Max Reitz> --- > blockdev.c | 48 > qapi/block-core.json | 17 + > qmp-commands.hx | 37 + > 3 files changed, 102 insertions(+) > > diff --git a/blockdev.c b/blockdev.c > index 481760a..a80d0e2 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -2164,6 +2164,54 @@ void qmp_blockdev_remove_medium(const char *device, > Error **errp) > } > } > > +static void qmp_blockdev_insert_anon_medium(const char *device, > +BlockDriverState *bs, Error > **errp) > +{ > +BlockBackend *blk; > +bool has_device; > + > +blk = blk_by_name(device); > +if (!blk) { > +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device); > +return; > +} > + > +/* For BBs without a device, we can exchange the BDS tree at will */ > +has_device = blk_get_attached_dev(blk); > + > +if (has_device && !blk_dev_has_removable_media(blk)) { > +error_setg(errp, "Device '%s' is not removable", device); > +return; > +} > + > +if (has_device && !blk_dev_is_tray_open(blk)) { > +error_setg(errp, "Tray of device '%s' is not open", device); > +return; > +} > + > +if (blk_bs(blk)) { > +error_setg(errp, "There already is a medium in device '%s'", device); > +return; > +} > + > +blk_insert_bs(blk, bs); > +} > + > +void qmp_blockdev_insert_medium(const char *device, const char *node_name, > +Error **errp) > +{ > +BlockDriverState *bs; > + > +bs = bdrv_find_node(node_name); > +if (!bs) { > +error_setg(errp, "Node '%s' not found", node_name); > +return; > +} > + > +qmp_blockdev_insert_anon_medium(device, bs, errp); > +} > + > /* throttling disk I/O limits */ > void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t > bps_rd, > int64_t bps_wr, > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 63a83e4..84c9b23 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -1925,6 +1925,23 @@ > { 'command': 'blockdev-remove-medium', >'data': { 'device': 'str' } } > > +## > +# @blockdev-insert-medium: > +# > +# Inserts a medium (a block driver state tree) into a block device. That > block > +# device's tray must currently be open and there must be no medium inserted > +# already. > +# > +# @device:block device name > +# > +# @node-name: name of a node in the block driver state graph > +# > +# Since: 2.5 > +## > +{ 'command': 'blockdev-insert-medium', > + 'data': { 'device': 'str', > +'node-name': 'str'} } > + > > ## > # @BlockErrorAction > diff --git a/qmp-commands.hx b/qmp-commands.hx > index ff6c572..b4c34fe 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -3991,6 +3991,43 @@ Example: > EQMP > > { > +.name = "blockdev-insert-medium", > +.args_type = "device:s,node-name:s", > +.mhandler.cmd_new = qmp_marshal_input_blockdev_insert_medium, > +}, > + > +SQMP > +blockdev-insert-medium > +-- > + > +Inserts a medium (a block driver state tree) into a block device. That block > +device's tray must currently be open and there must be no medium inserted > +already. > + > +Arguments: > + > +- "device": block device name (json-string) > +- "node-name": root node of the BDS tree to insert into the block device > + > +Example: > + > +-> { "execute": "blockdev-add", > + "arguments": { "options": { "node-name": "node0", > + "driver": "raw", > + "file": { "driver": "file", > + "filename": "fedora.iso" } } } } > + > +<- { "return": {} } > + > +-> { "execute": "blockdev-insert-medium", > + "arguments": { "device": "ide1-cd0", > +"node-name": "node0" } } > + > +<- { "return": {} } > + > +EQMP > + > +{ > .name = "query-named-block-nodes", > .args_type = "", > .mhandler.cmd_new = qmp_marshal_input_query_named_block_nodes, >
[Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium
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). Signed-off-by: Max Reitz mre...@redhat.com --- blockdev.c | 48 qapi/block-core.json | 17 + qmp-commands.hx | 37 + 3 files changed, 102 insertions(+) diff --git a/blockdev.c b/blockdev.c index 481760a..a80d0e2 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2164,6 +2164,54 @@ void qmp_blockdev_remove_medium(const char *device, Error **errp) } } +static void qmp_blockdev_insert_anon_medium(const char *device, +BlockDriverState *bs, Error **errp) +{ +BlockBackend *blk; +bool has_device; + +blk = blk_by_name(device); +if (!blk) { +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, + Device '%s' not found, device); +return; +} + +/* For BBs without a device, we can exchange the BDS tree at will */ +has_device = blk_get_attached_dev(blk); + +if (has_device !blk_dev_has_removable_media(blk)) { +error_setg(errp, Device '%s' is not removable, device); +return; +} + +if (has_device !blk_dev_is_tray_open(blk)) { +error_setg(errp, Tray of device '%s' is not open, device); +return; +} + +if (blk_bs(blk)) { +error_setg(errp, There already is a medium in device '%s', device); +return; +} + +blk_insert_bs(blk, bs); +} + +void qmp_blockdev_insert_medium(const char *device, const char *node_name, +Error **errp) +{ +BlockDriverState *bs; + +bs = bdrv_find_node(node_name); +if (!bs) { +error_setg(errp, Node '%s' not found, node_name); +return; +} + +qmp_blockdev_insert_anon_medium(device, bs, errp); +} + /* throttling disk I/O limits */ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd, int64_t bps_wr, diff --git a/qapi/block-core.json b/qapi/block-core.json index 63a83e4..84c9b23 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1925,6 +1925,23 @@ { 'command': 'blockdev-remove-medium', 'data': { 'device': 'str' } } +## +# @blockdev-insert-medium: +# +# Inserts a medium (a block driver state tree) into a block device. That block +# device's tray must currently be open and there must be no medium inserted +# already. +# +# @device:block device name +# +# @node-name: name of a node in the block driver state graph +# +# Since: 2.5 +## +{ 'command': 'blockdev-insert-medium', + 'data': { 'device': 'str', +'node-name': 'str'} } + ## # @BlockErrorAction diff --git a/qmp-commands.hx b/qmp-commands.hx index ff6c572..b4c34fe 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3991,6 +3991,43 @@ Example: EQMP { +.name = blockdev-insert-medium, +.args_type = device:s,node-name:s, +.mhandler.cmd_new = qmp_marshal_input_blockdev_insert_medium, +}, + +SQMP +blockdev-insert-medium +-- + +Inserts a medium (a block driver state tree) into a block device. That block +device's tray must currently be open and there must be no medium inserted +already. + +Arguments: + +- device: block device name (json-string) +- node-name: root node of the BDS tree to insert into the block device + +Example: + +- { execute: blockdev-add, + arguments: { options: { node-name: node0, + driver: raw, + file: { driver: file, + filename: fedora.iso } } } } + +- { return: {} } + +- { execute: blockdev-insert-medium, + arguments: { device: ide1-cd0, +node-name: node0 } } + +- { return: {} } + +EQMP + +{ .name = query-named-block-nodes, .args_type = , .mhandler.cmd_new = qmp_marshal_input_query_named_block_nodes, -- 2.4.6