[PATCH v3 for 7.1 1/1] block: add 'force' parameter to 'blockdev-change-medium' command
'blockdev-change-medium' is a convinient wrapper for the following
sequence of commands:
* blockdev-open-tray
* blockdev-remove-medium
* blockdev-insert-medium
* blockdev-close-tray
and should be used f.e. to change ISO image inside the CD-ROM tray.
Though the guest could lock the tray and some linux guests like
CentOS 8.5 actually does that. In this case the execution if this
command results in the error like the following:
Device 'scsi0-0-1-0' is locked and force was not specified,
wait for tray to open and try again.
This situation is could be resolved 'blockdev-open-tray' by passing
flag 'force' inside. Thus is seems reasonable to add the same
capability for 'blockdev-change-medium' too.
Signed-off-by: Denis V. Lunev
Reviewed-by: Vladimir Sementsov-Ogievskiy
Acked-by: "Dr. David Alan Gilbert"
CC: Kevin Wolf
CC: Hanna Reitz
CC: Eric Blake
CC: Markus Armbruster
---
block/qapi-sysemu.c | 3 ++-
hmp-commands.hx | 11 +++
monitor/hmp-cmds.c | 4 +++-
qapi/block.json | 6 ++
ui/cocoa.m | 1 +
5 files changed, 19 insertions(+), 6 deletions(-)
Changes from v2:
- fixed parameter's order in changeDeviceMedia(). This is a VERY interesting
story, actually. Both versions of the patch (v2 & v3) compile silently.
In order to see the difference one needs to enable -Weverything compilation
option!
Changes from v1:
- added kludge to Objective C code
- simplified a bit call of do_open_tray() (thanks, Vova!)
- added record to hmp-command.hx
diff --git a/block/qapi-sysemu.c b/block/qapi-sysemu.c
index 8498402ad4..680c7ee342 100644
--- a/block/qapi-sysemu.c
+++ b/block/qapi-sysemu.c
@@ -318,6 +318,7 @@ void qmp_blockdev_change_medium(bool has_device, const char
*device,
bool has_id, const char *id,
const char *filename,
bool has_format, const char *format,
+bool has_force, bool force,
bool has_read_only,
BlockdevChangeReadOnlyMode read_only,
Error **errp)
@@ -380,7 +381,7 @@ void qmp_blockdev_change_medium(bool has_device, const char
*device,
rc = do_open_tray(has_device ? device : NULL,
has_id ? id : NULL,
- false, &err);
+ force, &err);
if (rc && rc != -ENOSYS) {
error_propagate(errp, err);
goto fail;
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8476277aa9..6ec593ea08 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -202,9 +202,9 @@ ERST
{
.name = "change",
-.args_type = "device:B,target:F,arg:s?,read-only-mode:s?",
-.params = "device filename [format [read-only-mode]]",
-.help = "change a removable medium, optional format",
+.args_type = "device:B,force:-f,target:F,arg:s?,read-only-mode:s?",
+.params = "device [-f] filename [format [read-only-mode]]",
+.help = "change a removable medium, optional format, use -f to
force the operation",
.cmd= hmp_change,
},
@@ -212,11 +212,14 @@ SRST
``change`` *device* *setting*
Change the configuration of a device.
- ``change`` *diskdevice* *filename* [*format* [*read-only-mode*]]
+ ``change`` *diskdevice* [-f] *filename* [*format* [*read-only-mode*]]
Change the medium for a removable disk device to point to *filename*. eg::
(qemu) change ide1-cd0 /path/to/some.iso
+``-f``
+ forces the operation even if the guest has locked the tray.
+
*format* is optional.
*read-only-mode* may be used to change the read-only status of the device.
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 634968498b..d8b98bed6c 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1472,6 +1472,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
const char *target = qdict_get_str(qdict, "target");
const char *arg = qdict_get_try_str(qdict, "arg");
const char *read_only = qdict_get_try_str(qdict, "read-only-mode");
+bool force = qdict_get_try_bool(qdict, "force", false);
BlockdevChangeReadOnlyMode read_only_mode = 0;
Error *err = NULL;
@@ -1508,7 +1509,8 @@ void hmp_change(Monitor *mon, const QDict *qdict)
}
qmp_blockdev_change_medium(true, device, false, NULL, target,
- !!arg, arg, !!read_only, read_only_mode,
+ !!arg, arg, true, force,
+ !!read_only, read_only_mode,
&err);
}
diff --git a/qapi/block.json b/qapi/block.json
index 82fcf2c914..3f100d4887 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -326,6 +326,11 @@
# @read-only-mode: change the read-only mode of the device; defaults
# to 'retain'
#
+# @force: if false (the d
Re: [PATCH] hw/nvme: fix narrowing conversion
On Apr 12 11:59, Dmitry Tikhov wrote:
> Since nlbas is of type int, it does not work with large namespace size
> values, e.g., 9 TB size of file backing namespace and 8 byte metadata
> with 4096 bytes lbasz gives negative nlbas value, which is later
> promoted to negative int64_t type value and results in negative
> ns->moff which breaks namespace
>
> Signed-off-by: Dmitry Tikhov
> ---
> hw/nvme/ns.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
> index 324f53ea0c..af6504fad2 100644
> --- a/hw/nvme/ns.c
> +++ b/hw/nvme/ns.c
> @@ -29,7 +29,8 @@ void nvme_ns_init_format(NvmeNamespace *ns)
> {
> NvmeIdNs *id_ns = &ns->id_ns;
> BlockDriverInfo bdi;
> -int npdg, nlbas, ret;
> +int npdg, ret;
> +int64_t nlbas;
>
> ns->lbaf = id_ns->lbaf[NVME_ID_NS_FLBAS_INDEX(id_ns->flbas)];
> ns->lbasz = 1 << ns->lbaf.ds;
> @@ -42,7 +43,7 @@ void nvme_ns_init_format(NvmeNamespace *ns)
> id_ns->ncap = id_ns->nsze;
> id_ns->nuse = id_ns->ncap;
>
> -ns->moff = (int64_t)nlbas << ns->lbaf.ds;
> +ns->moff = nlbas << ns->lbaf.ds;
>
> npdg = ns->blkconf.discard_granularity / ns->lbasz;
>
> --
> 2.35.1
>
Thanks Dmitry. Looks reasonable,
Reviewed-by: Klaus Jensen
signature.asc
Description: PGP signature
[PATCH for-7.1] hw/block/fdc-sysbus: Always mark sysbus floppy controllers as not having DMA
The sysbus floppy controllers (devices sysbus-fdc and sun-fdtwo)
don't support DMA. The core floppy controller code expects this to
be indicated by setting FDCtrl::dma_chann to -1. This used to be
done in the device instance_init functions sysbus_fdc_initfn() and
sun4m_fdc_initfn(), but in commit 1430759ec3e we refactored this code
and accidentally lost the setting of dma_chann.
For sysbus-fdc this has no ill effects because we were redundantly
also setting dma_chann in fdctrl_init_sysbus(), but for sun-fdtwo
this means that guests which try to enable DMA on the floppy
controller will cause QEMU to crash because FDCtrl::dma is NULL.
Set dma_chann to -1 in the common instance init, and remove the
redundant code in fdctrl_init_sysbus() that is also setting it.
There is a six-year-old FIXME comment in the jazz board code to the
effect that in theory it should support doing DMA via a custom DMA
controller. If anybody ever chooses to fix that they can do it by
adding support for setting both FDCtrl::dma_chann and FDCtrl::dma.
(A QOM link property 'dma-controller' on the sysbus device which can
be set to an instance of IsaDmaClass is probably the way to go.)
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/958
Signed-off-by: Peter Maydell
---
include/hw/block/fdc.h | 3 +--
hw/block/fdc-sysbus.c | 14 +++---
hw/mips/jazz.c | 2 +-
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
index 1ecca7cac7f..35248c08379 100644
--- a/include/hw/block/fdc.h
+++ b/include/hw/block/fdc.h
@@ -10,8 +10,7 @@
#define TYPE_ISA_FDC "isa-fdc"
void isa_fdc_init_drives(ISADevice *fdc, DriveInfo **fds);
-void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
-hwaddr mmio_base, DriveInfo **fds);
+void fdctrl_init_sysbus(qemu_irq irq, hwaddr mmio_base, DriveInfo **fds);
void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
DriveInfo **fds, qemu_irq *fdc_tc);
diff --git a/hw/block/fdc-sysbus.c b/hw/block/fdc-sysbus.c
index 57fc8773f12..6c22c3510da 100644
--- a/hw/block/fdc-sysbus.c
+++ b/hw/block/fdc-sysbus.c
@@ -94,8 +94,7 @@ static void fdctrl_handle_tc(void *opaque, int irq, int level)
trace_fdctrl_tc_pulse(level);
}
-void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
-hwaddr mmio_base, DriveInfo **fds)
+void fdctrl_init_sysbus(qemu_irq irq, hwaddr mmio_base, DriveInfo **fds)
{
FDCtrl *fdctrl;
DeviceState *dev;
@@ -105,7 +104,6 @@ void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
dev = qdev_new("sysbus-fdc");
sys = SYSBUS_FDC(dev);
fdctrl = &sys->state;
-fdctrl->dma_chann = dma_chann; /* FIXME */
sbd = SYS_BUS_DEVICE(dev);
sysbus_realize_and_unref(sbd, &error_fatal);
sysbus_connect_irq(sbd, 0, irq);
@@ -138,6 +136,16 @@ static void sysbus_fdc_common_instance_init(Object *obj)
FDCtrlSysBus *sys = SYSBUS_FDC(obj);
FDCtrl *fdctrl = &sys->state;
+/*
+ * DMA is not currently supported for sysbus floppy controllers.
+ * If we wanted to add support then probably the best approach is
+ * to have a QOM link property 'dma-controller' which the board
+ * code can set to an instance of IsaDmaClass, and an integer
+ * property 'dma-channel', so that we can set fdctrl->dma and
+ * fdctrl->dma_chann accordingly.
+ */
+fdctrl->dma_chann = -1;
+
qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
memory_region_init_io(&fdctrl->iomem, obj,
diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
index 44f0d48bfd7..14eaa517435 100644
--- a/hw/mips/jazz.c
+++ b/hw/mips/jazz.c
@@ -354,7 +354,7 @@ static void mips_jazz_init(MachineState *machine,
fds[n] = drive_get(IF_FLOPPY, 0, n);
}
/* FIXME: we should enable DMA with a custom IsaDma device */
-fdctrl_init_sysbus(qdev_get_gpio_in(rc4030, 1), -1, 0x80003000, fds);
+fdctrl_init_sysbus(qdev_get_gpio_in(rc4030, 1), 0x80003000, fds);
/* Real time clock */
mc146818_rtc_init(isa_bus, 1980, NULL);
--
2.25.1
Re: [PATCH v2 for 7.1 1/1] block: add 'force' parameter to 'blockdev-change-medium' command
On 12.04.2022 16:17, Vladimir Sementsov-Ogievskiy wrote:
12.04.2022 12:50, Denis V. Lunev wrote:
'blockdev-change-medium' is a convinient wrapper for the following
sequence of commands:
* blockdev-open-tray
* blockdev-remove-medium
* blockdev-insert-medium
* blockdev-close-tray
and should be used f.e. to change ISO image inside the CD-ROM tray.
Though the guest could lock the tray and some linux guests like
CentOS 8.5 actually does that. In this case the execution if this
command results in the error like the following:
Device 'scsi0-0-1-0' is locked and force was not specified,
wait for tray to open and try again.
This situation is could be resolved 'blockdev-open-tray' by passing
flag 'force' inside. Thus is seems reasonable to add the same
capability for 'blockdev-change-medium' too.
Signed-off-by: Denis V. Lunev
CC: Kevin Wolf
CC: Hanna Reitz
CC: "Dr. David Alan Gilbert"
CC: Eric Blake
CC: Markus Armbruster
CC: Vladimir Sementsov-Ogievskiy
---
block/qapi-sysemu.c | 3 ++-
hmp-commands.hx | 11 +++
monitor/hmp-cmds.c | 4 +++-
qapi/block.json | 6 ++
ui/cocoa.m | 1 +
5 files changed, 19 insertions(+), 6 deletions(-)
Changes from v1:
- added kludge to Objective C code
- simplified a bit call of do_open_tray() (thanks, Vova!)
- added record to hmp-command.hx
diff --git a/block/qapi-sysemu.c b/block/qapi-sysemu.c
index 8498402ad4..680c7ee342 100644
--- a/block/qapi-sysemu.c
+++ b/block/qapi-sysemu.c
@@ -318,6 +318,7 @@ void qmp_blockdev_change_medium(bool has_device,
const char *device,
bool has_id, const char *id,
const char *filename,
bool has_format, const char *format,
+ bool has_force, bool force,
bool has_read_only,
BlockdevChangeReadOnlyMode read_only,
Error **errp)
@@ -380,7 +381,7 @@ void qmp_blockdev_change_medium(bool has_device,
const char *device,
rc = do_open_tray(has_device ? device : NULL,
has_id ? id : NULL,
- false, &err);
+ force, &err);
if (rc && rc != -ENOSYS) {
error_propagate(errp, err);
goto fail;
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8476277aa9..6ec593ea08 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -202,9 +202,9 @@ ERST
{
.name = "change",
- .args_type = "device:B,target:F,arg:s?,read-only-mode:s?",
- .params = "device filename [format [read-only-mode]]",
- .help = "change a removable medium, optional format",
+ .args_type =
"device:B,force:-f,target:F,arg:s?,read-only-mode:s?",
+ .params = "device [-f] filename [format [read-only-mode]]",
+ .help = "change a removable medium, optional format,
use -f to force the operation",
.cmd = hmp_change,
},
@@ -212,11 +212,14 @@ SRST
``change`` *device* *setting*
Change the configuration of a device.
- ``change`` *diskdevice* *filename* [*format* [*read-only-mode*]]
+ ``change`` *diskdevice* [-f] *filename* [*format* [*read-only-mode*]]
Change the medium for a removable disk device to point to
*filename*. eg::
(qemu) change ide1-cd0 /path/to/some.iso
+ ``-f``
+ forces the operation even if the guest has locked the tray.
+
*format* is optional.
*read-only-mode* may be used to change the read-only status
of the device.
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 634968498b..d8b98bed6c 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1472,6 +1472,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
const char *target = qdict_get_str(qdict, "target");
const char *arg = qdict_get_try_str(qdict, "arg");
const char *read_only = qdict_get_try_str(qdict,
"read-only-mode");
+ bool force = qdict_get_try_bool(qdict, "force", false);
BlockdevChangeReadOnlyMode read_only_mode = 0;
Error *err = NULL;
@@ -1508,7 +1509,8 @@ void hmp_change(Monitor *mon, const QDict
*qdict)
}
qmp_blockdev_change_medium(true, device, false, NULL,
target,
- !!arg, arg, !!read_only,
read_only_mode,
+ !!arg, arg, true, force,
+ !!read_only, read_only_mode,
&err);
}
diff --git a/qapi/block.json b/qapi/block.json
index 82fcf2c914..3f100d4887 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -326,6 +326,11 @@
# @read-only-mode: change the read-only mode of the device; defaults
# to 'retain'
#
+# @force: if false (the default), an eject request through
blockdev-open-tray
+# will be sent to the guest if it has locked the
Re: [PATCH v2 for 7.1 1/1] block: add 'force' parameter to 'blockdev-change-medium' command
12.04.2022 12:50, Denis V. Lunev wrote:
'blockdev-change-medium' is a convinient wrapper for the following
sequence of commands:
* blockdev-open-tray
* blockdev-remove-medium
* blockdev-insert-medium
* blockdev-close-tray
and should be used f.e. to change ISO image inside the CD-ROM tray.
Though the guest could lock the tray and some linux guests like
CentOS 8.5 actually does that. In this case the execution if this
command results in the error like the following:
Device 'scsi0-0-1-0' is locked and force was not specified,
wait for tray to open and try again.
This situation is could be resolved 'blockdev-open-tray' by passing
flag 'force' inside. Thus is seems reasonable to add the same
capability for 'blockdev-change-medium' too.
Signed-off-by: Denis V. Lunev
CC: Kevin Wolf
CC: Hanna Reitz
CC: "Dr. David Alan Gilbert"
CC: Eric Blake
CC: Markus Armbruster
CC: Vladimir Sementsov-Ogievskiy
---
block/qapi-sysemu.c | 3 ++-
hmp-commands.hx | 11 +++
monitor/hmp-cmds.c | 4 +++-
qapi/block.json | 6 ++
ui/cocoa.m | 1 +
5 files changed, 19 insertions(+), 6 deletions(-)
Changes from v1:
- added kludge to Objective C code
- simplified a bit call of do_open_tray() (thanks, Vova!)
- added record to hmp-command.hx
diff --git a/block/qapi-sysemu.c b/block/qapi-sysemu.c
index 8498402ad4..680c7ee342 100644
--- a/block/qapi-sysemu.c
+++ b/block/qapi-sysemu.c
@@ -318,6 +318,7 @@ void qmp_blockdev_change_medium(bool has_device, const char
*device,
bool has_id, const char *id,
const char *filename,
bool has_format, const char *format,
+bool has_force, bool force,
bool has_read_only,
BlockdevChangeReadOnlyMode read_only,
Error **errp)
@@ -380,7 +381,7 @@ void qmp_blockdev_change_medium(bool has_device, const char
*device,
rc = do_open_tray(has_device ? device : NULL,
has_id ? id : NULL,
- false, &err);
+ force, &err);
if (rc && rc != -ENOSYS) {
error_propagate(errp, err);
goto fail;
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8476277aa9..6ec593ea08 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -202,9 +202,9 @@ ERST
{
.name = "change",
-.args_type = "device:B,target:F,arg:s?,read-only-mode:s?",
-.params = "device filename [format [read-only-mode]]",
-.help = "change a removable medium, optional format",
+.args_type = "device:B,force:-f,target:F,arg:s?,read-only-mode:s?",
+.params = "device [-f] filename [format [read-only-mode]]",
+.help = "change a removable medium, optional format, use -f to force
the operation",
.cmd= hmp_change,
},
@@ -212,11 +212,14 @@ SRST
``change`` *device* *setting*
Change the configuration of a device.
- ``change`` *diskdevice* *filename* [*format* [*read-only-mode*]]
+ ``change`` *diskdevice* [-f] *filename* [*format* [*read-only-mode*]]
Change the medium for a removable disk device to point to *filename*. eg::
(qemu) change ide1-cd0 /path/to/some.iso
+``-f``
+ forces the operation even if the guest has locked the tray.
+
*format* is optional.
*read-only-mode* may be used to change the read-only status of the device.
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 634968498b..d8b98bed6c 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1472,6 +1472,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
const char *target = qdict_get_str(qdict, "target");
const char *arg = qdict_get_try_str(qdict, "arg");
const char *read_only = qdict_get_try_str(qdict, "read-only-mode");
+bool force = qdict_get_try_bool(qdict, "force", false);
BlockdevChangeReadOnlyMode read_only_mode = 0;
Error *err = NULL;
@@ -1508,7 +1509,8 @@ void hmp_change(Monitor *mon, const QDict *qdict)
}
qmp_blockdev_change_medium(true, device, false, NULL, target,
- !!arg, arg, !!read_only, read_only_mode,
+ !!arg, arg, true, force,
+ !!read_only, read_only_mode,
&err);
}
diff --git a/qapi/block.json b/qapi/block.json
index 82fcf2c914..3f100d4887 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -326,6 +326,11 @@
# @read-only-mode: change the read-only mode of the device; defaults
# to 'retain'
#
+# @force: if false (the default), an eject request through blockdev-open-tray
+# will be sent to the guest if it has locked the tray (and the tray
+# will not be opened immedi
[PATCH] hw/nvme: fix narrowing conversion
Since nlbas is of type int, it does not work with large namespace size
values, e.g., 9 TB size of file backing namespace and 8 byte metadata
with 4096 bytes lbasz gives negative nlbas value, which is later
promoted to negative int64_t type value and results in negative
ns->moff which breaks namespace
Signed-off-by: Dmitry Tikhov
---
hw/nvme/ns.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index 324f53ea0c..af6504fad2 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -29,7 +29,8 @@ void nvme_ns_init_format(NvmeNamespace *ns)
{
NvmeIdNs *id_ns = &ns->id_ns;
BlockDriverInfo bdi;
-int npdg, nlbas, ret;
+int npdg, ret;
+int64_t nlbas;
ns->lbaf = id_ns->lbaf[NVME_ID_NS_FLBAS_INDEX(id_ns->flbas)];
ns->lbasz = 1 << ns->lbaf.ds;
@@ -42,7 +43,7 @@ void nvme_ns_init_format(NvmeNamespace *ns)
id_ns->ncap = id_ns->nsze;
id_ns->nuse = id_ns->ncap;
-ns->moff = (int64_t)nlbas << ns->lbaf.ds;
+ns->moff = nlbas << ns->lbaf.ds;
npdg = ns->blkconf.discard_granularity / ns->lbasz;
--
2.35.1
Re: [PATCH v2 for 7.1 1/1] block: add 'force' parameter to 'blockdev-change-medium' command
* Denis V. Lunev (d...@openvz.org) wrote:
> 'blockdev-change-medium' is a convinient wrapper for the following
> sequence of commands:
> * blockdev-open-tray
> * blockdev-remove-medium
> * blockdev-insert-medium
> * blockdev-close-tray
> and should be used f.e. to change ISO image inside the CD-ROM tray.
> Though the guest could lock the tray and some linux guests like
> CentOS 8.5 actually does that. In this case the execution if this
> command results in the error like the following:
> Device 'scsi0-0-1-0' is locked and force was not specified,
> wait for tray to open and try again.
>
> This situation is could be resolved 'blockdev-open-tray' by passing
> flag 'force' inside. Thus is seems reasonable to add the same
> capability for 'blockdev-change-medium' too.
For HMP:
Acked-by: Dr. David Alan Gilbert
(Although I'd be pretty careful with this; a guest OS might feel like
it could ignore anything else that was going on and keep it's data
cached if it had it's drive locked).
Dave
> Signed-off-by: Denis V. Lunev
> CC: Kevin Wolf
> CC: Hanna Reitz
> CC: "Dr. David Alan Gilbert"
> CC: Eric Blake
> CC: Markus Armbruster
> CC: Vladimir Sementsov-Ogievskiy
> ---
> block/qapi-sysemu.c | 3 ++-
> hmp-commands.hx | 11 +++
> monitor/hmp-cmds.c | 4 +++-
> qapi/block.json | 6 ++
> ui/cocoa.m | 1 +
> 5 files changed, 19 insertions(+), 6 deletions(-)
>
> Changes from v1:
> - added kludge to Objective C code
> - simplified a bit call of do_open_tray() (thanks, Vova!)
> - added record to hmp-command.hx
>
> diff --git a/block/qapi-sysemu.c b/block/qapi-sysemu.c
> index 8498402ad4..680c7ee342 100644
> --- a/block/qapi-sysemu.c
> +++ b/block/qapi-sysemu.c
> @@ -318,6 +318,7 @@ void qmp_blockdev_change_medium(bool has_device, const
> char *device,
> bool has_id, const char *id,
> const char *filename,
> bool has_format, const char *format,
> +bool has_force, bool force,
> bool has_read_only,
> BlockdevChangeReadOnlyMode read_only,
> Error **errp)
> @@ -380,7 +381,7 @@ void qmp_blockdev_change_medium(bool has_device, const
> char *device,
>
> rc = do_open_tray(has_device ? device : NULL,
>has_id ? id : NULL,
> - false, &err);
> + force, &err);
> if (rc && rc != -ENOSYS) {
> error_propagate(errp, err);
> goto fail;
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 8476277aa9..6ec593ea08 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -202,9 +202,9 @@ ERST
>
> {
> .name = "change",
> -.args_type = "device:B,target:F,arg:s?,read-only-mode:s?",
> -.params = "device filename [format [read-only-mode]]",
> -.help = "change a removable medium, optional format",
> +.args_type = "device:B,force:-f,target:F,arg:s?,read-only-mode:s?",
> +.params = "device [-f] filename [format [read-only-mode]]",
> +.help = "change a removable medium, optional format, use -f to
> force the operation",
> .cmd= hmp_change,
> },
>
> @@ -212,11 +212,14 @@ SRST
> ``change`` *device* *setting*
>Change the configuration of a device.
>
> - ``change`` *diskdevice* *filename* [*format* [*read-only-mode*]]
> + ``change`` *diskdevice* [-f] *filename* [*format* [*read-only-mode*]]
> Change the medium for a removable disk device to point to *filename*.
> eg::
>
>(qemu) change ide1-cd0 /path/to/some.iso
>
> +``-f``
> + forces the operation even if the guest has locked the tray.
> +
> *format* is optional.
>
> *read-only-mode* may be used to change the read-only status of the
> device.
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 634968498b..d8b98bed6c 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1472,6 +1472,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
> const char *target = qdict_get_str(qdict, "target");
> const char *arg = qdict_get_try_str(qdict, "arg");
> const char *read_only = qdict_get_try_str(qdict, "read-only-mode");
> +bool force = qdict_get_try_bool(qdict, "force", false);
> BlockdevChangeReadOnlyMode read_only_mode = 0;
> Error *err = NULL;
>
> @@ -1508,7 +1509,8 @@ void hmp_change(Monitor *mon, const QDict *qdict)
> }
>
> qmp_blockdev_change_medium(true, device, false, NULL, target,
> - !!arg, arg, !!read_only, read_only_mode,
> + !!arg, arg, true, force,
> + !!read_only, read_only_mode,
> &err);
> }
>
> diff --git a/qapi/block.json b/qapi/block.jso
[PATCH v2 for 7.1 1/1] block: add 'force' parameter to 'blockdev-change-medium' command
'blockdev-change-medium' is a convinient wrapper for the following
sequence of commands:
* blockdev-open-tray
* blockdev-remove-medium
* blockdev-insert-medium
* blockdev-close-tray
and should be used f.e. to change ISO image inside the CD-ROM tray.
Though the guest could lock the tray and some linux guests like
CentOS 8.5 actually does that. In this case the execution if this
command results in the error like the following:
Device 'scsi0-0-1-0' is locked and force was not specified,
wait for tray to open and try again.
This situation is could be resolved 'blockdev-open-tray' by passing
flag 'force' inside. Thus is seems reasonable to add the same
capability for 'blockdev-change-medium' too.
Signed-off-by: Denis V. Lunev
CC: Kevin Wolf
CC: Hanna Reitz
CC: "Dr. David Alan Gilbert"
CC: Eric Blake
CC: Markus Armbruster
CC: Vladimir Sementsov-Ogievskiy
---
block/qapi-sysemu.c | 3 ++-
hmp-commands.hx | 11 +++
monitor/hmp-cmds.c | 4 +++-
qapi/block.json | 6 ++
ui/cocoa.m | 1 +
5 files changed, 19 insertions(+), 6 deletions(-)
Changes from v1:
- added kludge to Objective C code
- simplified a bit call of do_open_tray() (thanks, Vova!)
- added record to hmp-command.hx
diff --git a/block/qapi-sysemu.c b/block/qapi-sysemu.c
index 8498402ad4..680c7ee342 100644
--- a/block/qapi-sysemu.c
+++ b/block/qapi-sysemu.c
@@ -318,6 +318,7 @@ void qmp_blockdev_change_medium(bool has_device, const char
*device,
bool has_id, const char *id,
const char *filename,
bool has_format, const char *format,
+bool has_force, bool force,
bool has_read_only,
BlockdevChangeReadOnlyMode read_only,
Error **errp)
@@ -380,7 +381,7 @@ void qmp_blockdev_change_medium(bool has_device, const char
*device,
rc = do_open_tray(has_device ? device : NULL,
has_id ? id : NULL,
- false, &err);
+ force, &err);
if (rc && rc != -ENOSYS) {
error_propagate(errp, err);
goto fail;
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8476277aa9..6ec593ea08 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -202,9 +202,9 @@ ERST
{
.name = "change",
-.args_type = "device:B,target:F,arg:s?,read-only-mode:s?",
-.params = "device filename [format [read-only-mode]]",
-.help = "change a removable medium, optional format",
+.args_type = "device:B,force:-f,target:F,arg:s?,read-only-mode:s?",
+.params = "device [-f] filename [format [read-only-mode]]",
+.help = "change a removable medium, optional format, use -f to
force the operation",
.cmd= hmp_change,
},
@@ -212,11 +212,14 @@ SRST
``change`` *device* *setting*
Change the configuration of a device.
- ``change`` *diskdevice* *filename* [*format* [*read-only-mode*]]
+ ``change`` *diskdevice* [-f] *filename* [*format* [*read-only-mode*]]
Change the medium for a removable disk device to point to *filename*. eg::
(qemu) change ide1-cd0 /path/to/some.iso
+``-f``
+ forces the operation even if the guest has locked the tray.
+
*format* is optional.
*read-only-mode* may be used to change the read-only status of the device.
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 634968498b..d8b98bed6c 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1472,6 +1472,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
const char *target = qdict_get_str(qdict, "target");
const char *arg = qdict_get_try_str(qdict, "arg");
const char *read_only = qdict_get_try_str(qdict, "read-only-mode");
+bool force = qdict_get_try_bool(qdict, "force", false);
BlockdevChangeReadOnlyMode read_only_mode = 0;
Error *err = NULL;
@@ -1508,7 +1509,8 @@ void hmp_change(Monitor *mon, const QDict *qdict)
}
qmp_blockdev_change_medium(true, device, false, NULL, target,
- !!arg, arg, !!read_only, read_only_mode,
+ !!arg, arg, true, force,
+ !!read_only, read_only_mode,
&err);
}
diff --git a/qapi/block.json b/qapi/block.json
index 82fcf2c914..3f100d4887 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -326,6 +326,11 @@
# @read-only-mode: change the read-only mode of the device; defaults
# to 'retain'
#
+# @force: if false (the default), an eject request through blockdev-open-tray
+# will be sent to the guest if it has locked the tray (and the tray
+# will not be opened immediately); if true, the tray will be opened
+# regardless of whether it is locked. (since 7.1)
+#
#
Re: [RFC v2 1/8] blkio: add io_uring block driver using libblkio
On Thu, Apr 07, 2022 at 10:34:07AM +0200, Kevin Wolf wrote:
> Am 07.04.2022 um 10:25 hat Kevin Wolf geschrieben:
> > Am 07.04.2022 um 09:22 hat Stefan Hajnoczi geschrieben:
> > > On Wed, Apr 06, 2022 at 07:32:04PM +0200, Kevin Wolf wrote:
> > > > Am 05.04.2022 um 17:33 hat Stefan Hajnoczi geschrieben:
> > > > > libblkio (https://gitlab.com/libblkio/libblkio/) is a library for
> > > > > high-performance disk I/O. It currently supports io_uring with
> > > > > additional drivers planned.
> > > > >
> > > > > One of the reasons for developing libblkio is that other applications
> > > > > besides QEMU can use it. This will be particularly useful for
> > > > > vhost-user-blk which applications may wish to use for connecting to
> > > > > qemu-storage-daemon.
> > > > >
> > > > > libblkio also gives us an opportunity to develop in Rust behind a C
> > > > > API
> > > > > that is easy to consume from QEMU.
> > > > >
> > > > > This commit adds an io_uring BlockDriver to QEMU using libblkio. For
> > > > > now
> > > > > I/O buffers are copied through bounce buffers if the libblkio driver
> > > > > requires it. Later commits add an optimization for pre-registering
> > > > > guest
> > > > > RAM to avoid bounce buffers. It will be easy to add other libblkio
> > > > > drivers since they will share the majority of code.
> > > > >
> > > > > Signed-off-by: Stefan Hajnoczi
> > > >
> > > > > +static BlockDriver bdrv_io_uring = {
> > > > > +.format_name= "io_uring",
> > > > > +.protocol_name = "io_uring",
> > > > > +.instance_size = sizeof(BDRVBlkioState),
> > > > > +.bdrv_needs_filename= true,
> > > > > +.bdrv_parse_filename= blkio_parse_filename_io_uring,
> > > > > +.bdrv_file_open = blkio_file_open,
> > > > > +.bdrv_close = blkio_close,
> > > > > +.bdrv_getlength = blkio_getlength,
> > > > > +.has_variable_length= true,
> > > >
> > > > This one is a bad idea. It means that every request will call
> > > > blkio_getlength() first, which looks up the "capacity" property in
> > > > libblkio and then calls lseek() for the io_uring backend.
> > >
> > > Thanks for pointing this out. I didn't think this through. More below on
> > > what I was trying to do.
> > >
> > > > For other backends like the vhost_user one (where I just copied your
> > > > definition and then noticed this behaviour), it involve a message over
> > > > the vhost socket, which is even worse.
> > >
> > > (A vhost-user-blk driver could cache the capacity field and update it
> > > when a Configuration Change Notification is received. There is no need
> > > to send a vhost-user protocol message every time.)
> >
> > In theory we could cache in libblkio, but then we would need a mechanism
> > to invalidate the cache so we can support resizing an image (similar to
> > what block_resize does in QEMU, except that it wouldn't set the new
> > size from a parameter, but just get the new value from the backend).
>
> Oh, sorry, I misread. VHOST_USER_SLAVE_CONFIG_CHANGE_MSG is probably
> what you mean, so that the backend triggers the update. It exists in the
> spec, but neither libvhost-user nor rust-vmm seem to support it
> currently. We also don't set up the backchannel yet where this message
> could even be passed.
>
> So it's an option, but probably only for later because it involves
> extending several places.
Agreed.
Stefan
signature.asc
Description: PGP signature
Re: [PATCH 0/3] vhost-user: Fixes for VHOST_USER_ADD/REM_MEM_REG
On Thu, Apr 07, 2022 at 03:36:54PM +0200, Kevin Wolf wrote: > While implementing a vhost-user-blk driver for libblkio, I found some > problems with VHOST_USER_ADD/REM_MEM_REG both in the spec and in the > implementations in QEMU and libvhost-user that this series addresses. > > I also noticed that you can use REM_MEM_REG or SET_MEM_TABLE to unmap a > memory region that is still in use (e.g. a block I/O request using > addresses from the region has been started, but not completed yet), > which is not great. I'm not sure how to fix this best, though. > > We would have to wait for these requests to complete (maybe introduce a > refcount and wait for it to drop to zero), but waiting seems impossible > in libvhost-user because it doesn't have any main loop integration. Just > failing the memory region removal would be safe, but potentially a > rather awkward interface because clients would have to implement some > retry logic. > > Kevin Wolf (3): > docs/vhost-user: Clarifications for VHOST_USER_ADD/REM_MEM_REG > libvhost-user: Fix extra vu_add/rem_mem_reg reply > vhost-user: Don't pass file descriptor for VHOST_USER_REM_MEM_REG > > docs/interop/vhost-user.rst | 17 + > hw/virtio/vhost-user.c| 2 +- > subprojects/libvhost-user/libvhost-user.c | 17 +++-- > 3 files changed, 25 insertions(+), 11 deletions(-) > > -- > 2.35.1 > Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
