* [Qemu-devel] [PATCH v2 for-2.4 0/3] block: Improve warnings for doubly-connected drives
@ 2015-06-23 14:01 Peter Maydell
2015-06-23 14:01 ` [Qemu-devel] [PATCH v2 for-2.4 1/3] qdev-properties-system: Change set_pointer's parse callback to use Error Peter Maydell
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Peter Maydell @ 2015-06-23 14:01 UTC (permalink / raw
To: qemu-devel; +Cc: Kevin Wolf, Markus Armbruster, qemu-block, patches
This patchset attempts to improve the warning and error messages for
bad user command lines that attempt to connect a drive up to two
devices. The motivation here is patch #4, which changes the default
interface for the virt board to virtio. That will break some existing
command lines which forgot to specify if=none, and so I would like
us to at least diagnose that user error in a helpful way that points
the user towards adding the missing if=none.
Version 2 reduces scope, because it turns out that it is harder than
we thought to identify whether a use of an if=something drive is the
auto-plugging or a manual wiring up to a device. So all we do here
is improve the error messages for two situations which were already
errors but with rather cryptic messages:
(1) Drive specified as to be auto-connected and also manually connected
(and the board does handle this if= type):
qemu-system-x86_64 -nodefaults -display none \
-drive if=scsi,file=tmp.qcow2,id=foo -device ide-hd,drive=foo
Previously:
qemu-system-x86_64: -device ide-hd,drive=foo: Property 'ide-hd.drive'
can't take value 'foo', it's in use
Now:
qemu-system-x86_64: -device ide-hd,drive=foo: Drive 'foo' is already in
use because it has been automatically connected to another device (did
you need 'if=none' in the drive options?)
(2) Drive specified to be manually connected in two different ways:
qemu-system-x86_64 -nodefaults -display none \
-drive if=none,file=tmp.qcow2,id=foo -device ide-hd,drive=foo \
-device ide-hd,drive=foo
Previously:
qemu-system-x86_64: -device ide-hd,drive=foo: Property 'ide-hd.drive'
can't take value 'foo', it's in use
Now:
qemu-system-x86_64: -device ide-hd,drive=foo: Drive 'foo' is already in
use by another device
(We'll also produce message 1 in the oddball case where the user creates
a drive if=something-not-handled-by-machine and then wires it up manually
to two different devices; in this case their command line is doubly broken
and if they use if=none as suggested by message 1 they'll then get message 2
and can fix their own double-usage...)
Changes v1->v2:
* drop "warn if an if=<something> drive was also connected manually" patch
* change implementation of "improve error message" patch
Peter Maydell (3):
qdev-properties-system: Change set_pointer's parse callback to use
Error
qdev-properties-system: Improve error message for drive assignment
conflict
hw/arm/virt: Make block devices default to virtio
hw/arm/virt.c | 2 ++
hw/core/qdev-properties-system.c | 42 +++++++++++++++++++++++++++-------------
2 files changed, 31 insertions(+), 13 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.4 1/3] qdev-properties-system: Change set_pointer's parse callback to use Error
2015-06-23 14:01 [Qemu-devel] [PATCH v2 for-2.4 0/3] block: Improve warnings for doubly-connected drives Peter Maydell
@ 2015-06-23 14:01 ` Peter Maydell
2015-06-23 14:01 ` [Qemu-devel] [PATCH v2 for-2.4 2/3] qdev-properties-system: Improve error message for drive assignment conflict Peter Maydell
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2015-06-23 14:01 UTC (permalink / raw
To: qemu-devel; +Cc: Kevin Wolf, Markus Armbruster, qemu-block, patches
Instead of having set_pointer() call a parse callback which returns
an error number that we then convert to an Error string with
error_set_from_qdev_prop_error(), make the parse callback take an
Error** and set the error itself. This will allow parse routines
to provide more helpful error messages than the generic ones.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/core/qdev-properties-system.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 0309fe5..56954b4 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -35,15 +35,15 @@ static void get_pointer(Object *obj, Visitor *v, Property *prop,
}
static void set_pointer(Object *obj, Visitor *v, Property *prop,
- int (*parse)(DeviceState *dev, const char *str,
- void **ptr),
+ void (*parse)(DeviceState *dev, const char *str,
+ void **ptr, const char *propname,
+ Error **errp),
const char *name, Error **errp)
{
DeviceState *dev = DEVICE(obj);
Error *local_err = NULL;
void **ptr = qdev_get_prop_ptr(dev, prop);
char *str;
- int ret;
if (dev->realized) {
qdev_prop_set_after_realize(dev, name, errp);
@@ -60,26 +60,29 @@ static void set_pointer(Object *obj, Visitor *v, Property *prop,
*ptr = NULL;
return;
}
- ret = parse(dev, str, ptr);
- error_set_from_qdev_prop_error(errp, ret, dev, prop, str);
+ parse(dev, str, ptr, prop->name, errp);
g_free(str);
}
/* --- drive --- */
-static int parse_drive(DeviceState *dev, const char *str, void **ptr)
+static void parse_drive(DeviceState *dev, const char *str, void **ptr,
+ const char *propname, Error **errp)
{
BlockBackend *blk;
blk = blk_by_name(str);
if (!blk) {
- return -ENOENT;
+ error_setg(errp, "Property '%s.%s' can't find value '%s'",
+ object_get_typename(OBJECT(dev)), propname, str);
+ return;
}
if (blk_attach_dev(blk, dev) < 0) {
- return -EEXIST;
+ error_setg(errp, "Property '%s.%s' can't take value '%s', it's in use",
+ object_get_typename(OBJECT(dev)), propname, str);
+ return;
}
*ptr = blk;
- return 0;
}
static void release_drive(Object *obj, const char *name, void *opaque)
@@ -121,17 +124,21 @@ PropertyInfo qdev_prop_drive = {
/* --- character device --- */
-static int parse_chr(DeviceState *dev, const char *str, void **ptr)
+static void parse_chr(DeviceState *dev, const char *str, void **ptr,
+ const char *propname, Error **errp)
{
CharDriverState *chr = qemu_chr_find(str);
if (chr == NULL) {
- return -ENOENT;
+ error_setg(errp, "Property '%s.%s' can't find value '%s'",
+ object_get_typename(OBJECT(dev)), propname, str);
+ return;
}
if (qemu_chr_fe_claim(chr) != 0) {
- return -EEXIST;
+ error_setg(errp, "Property '%s.%s' can't take value '%s', it's in use",
+ object_get_typename(OBJECT(dev)), propname, str);
+ return;
}
*ptr = chr;
- return 0;
}
static void release_chr(Object *obj, const char *name, void *opaque)
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.4 2/3] qdev-properties-system: Improve error message for drive assignment conflict
2015-06-23 14:01 [Qemu-devel] [PATCH v2 for-2.4 0/3] block: Improve warnings for doubly-connected drives Peter Maydell
2015-06-23 14:01 ` [Qemu-devel] [PATCH v2 for-2.4 1/3] qdev-properties-system: Change set_pointer's parse callback to use Error Peter Maydell
@ 2015-06-23 14:01 ` Peter Maydell
2015-06-23 14:01 ` [Qemu-devel] [PATCH v2 for-2.4 3/3] hw/arm/virt: Make block devices default to virtio Peter Maydell
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2015-06-23 14:01 UTC (permalink / raw
To: qemu-devel; +Cc: Kevin Wolf, Markus Armbruster, qemu-block, patches
If the user forgot if=none on their drive specification they're likely
to get an error message because the drive is assigned once automatically
by QEMU and once by the manual id=/drive= user command line specification.
Improve the error message produced in this case to explicitly guide the
user towards if=none.
We rephrase the "drive conflict but not for an if=something" error as
well to keep the wording in line.
The two cases that change are:
(1) Drive specified as to be auto-connected and also manually connected
(and the board does handle this if= type):
qemu-system-x86_64 -nodefaults -display none \
-drive if=scsi,file=tmp.qcow2,id=foo -device ide-hd,drive=foo
Previously:
qemu-system-x86_64: -device ide-hd,drive=foo: Property 'ide-hd.drive'
can't take value 'foo', it's in use
Now:
qemu-system-x86_64: -device ide-hd,drive=foo: Drive 'foo' is already in
use because it has been automatically connected to another device (did
you need 'if=none' in the drive options?)
(2) Drive specified to be manually connected in two different ways:
qemu-system-x86_64 -nodefaults -display none \
-drive if=none,file=tmp.qcow2,id=foo -device ide-hd,drive=foo \
-device ide-hd,drive=foo
Previously:
qemu-system-x86_64: -device ide-hd,drive=foo: Property 'ide-hd.drive'
can't take value 'foo', it's in use
Now:
qemu-system-x86_64: -device ide-hd,drive=foo: Drive 'foo' is already in
use by another device
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/core/qdev-properties-system.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 56954b4..820cd14 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -78,8 +78,17 @@ static void parse_drive(DeviceState *dev, const char *str, void **ptr,
return;
}
if (blk_attach_dev(blk, dev) < 0) {
- error_setg(errp, "Property '%s.%s' can't take value '%s', it's in use",
- object_get_typename(OBJECT(dev)), propname, str);
+ DriveInfo *dinfo = blk_legacy_dinfo(blk);
+
+ if (dinfo->type != IF_NONE) {
+ error_setg(errp, "Drive '%s' is already in use because "
+ "it has been automatically connected to another "
+ "device (did you need 'if=none' in the drive options?)",
+ str);
+ } else {
+ error_setg(errp, "Drive '%s' is already in use by another device",
+ str);
+ }
return;
}
*ptr = blk;
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.4 3/3] hw/arm/virt: Make block devices default to virtio
2015-06-23 14:01 [Qemu-devel] [PATCH v2 for-2.4 0/3] block: Improve warnings for doubly-connected drives Peter Maydell
2015-06-23 14:01 ` [Qemu-devel] [PATCH v2 for-2.4 1/3] qdev-properties-system: Change set_pointer's parse callback to use Error Peter Maydell
2015-06-23 14:01 ` [Qemu-devel] [PATCH v2 for-2.4 2/3] qdev-properties-system: Improve error message for drive assignment conflict Peter Maydell
@ 2015-06-23 14:01 ` Peter Maydell
2015-06-25 7:40 ` Markus Armbruster
2015-06-25 9:26 ` [Qemu-devel] [PATCH v2 for-2.4 0/3] block: Improve warnings for doubly-connected drives Markus Armbruster
2015-06-25 13:23 ` [Qemu-devel] " Stefan Hajnoczi
4 siblings, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2015-06-23 14:01 UTC (permalink / raw
To: qemu-devel; +Cc: Kevin Wolf, Markus Armbruster, qemu-block, patches
Now we have virtio-pci, we can make the virt board's default block
device type be IF_VIRTIO. This allows users to use simplified
command lines that don't have to explicitly create virtio-pci-blk
devices; the -hda &c very short options now also work.
This means we also need to set no_cdrom to avoid getting a
default cdrom device -- this is needed because the virtio-blk
device will fail if it is connected to a block backend with
no media, which is what the default cdrom device typically is.
Providing a cdrom with media via -cdrom will still work.
Note that this change means that some command lines which used
to work (by accident) will stop working. Where a drive was connected
manually to a device but without 'if=none' being specified, we
used to treat this as an IDE drive, which we would then not autoplug
because the board doesn't support IDE. Now we will treat it as a
virtio disk and autoplug it, which means the attempt to use the
drive manually will fail:
qemu-system-arm: -drive file=img.qcow2,id=foo: Drive 'foo' is already
in use because it has been automatically connected to another device
(did you need 'if=none' in the drive options?)
The command line will be changed to include 'if=none', as the
error message suggests.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/virt.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index f1e85c8..7e643ba 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -966,6 +966,8 @@ static void virt_class_init(ObjectClass *oc, void *data)
mc->init = machvirt_init;
mc->max_cpus = 8;
mc->has_dynamic_sysbus = true;
+ mc->block_default_type = IF_VIRTIO;
+ mc->no_cdrom = 1;
}
static const TypeInfo machvirt_info = {
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.4 3/3] hw/arm/virt: Make block devices default to virtio
2015-06-23 14:01 ` [Qemu-devel] [PATCH v2 for-2.4 3/3] hw/arm/virt: Make block devices default to virtio Peter Maydell
@ 2015-06-25 7:40 ` Markus Armbruster
2015-06-25 9:04 ` Peter Maydell
0 siblings, 1 reply; 12+ messages in thread
From: Markus Armbruster @ 2015-06-25 7:40 UTC (permalink / raw
To: Peter Maydell; +Cc: Kevin Wolf, qemu-devel, qemu-block, patches
Peter Maydell <peter.maydell@linaro.org> writes:
> Now we have virtio-pci, we can make the virt board's default block
> device type be IF_VIRTIO. This allows users to use simplified
> command lines that don't have to explicitly create virtio-pci-blk
> devices; the -hda &c very short options now also work.
>
> This means we also need to set no_cdrom to avoid getting a
> default cdrom device -- this is needed because the virtio-blk
> device will fail if it is connected to a block backend with
> no media, which is what the default cdrom device typically is.
> Providing a cdrom with media via -cdrom will still work.
It'll create a virtio-blk device with non-removable medium, won't it?
> Note that this change means that some command lines which used
> to work (by accident) will stop working. Where a drive was connected
> manually to a device but without 'if=none' being specified, we
> used to treat this as an IDE drive, which we would then not autoplug
> because the board doesn't support IDE. Now we will treat it as a
> virtio disk and autoplug it, which means the attempt to use the
> drive manually will fail:
> qemu-system-arm: -drive file=img.qcow2,id=foo: Drive 'foo' is already
> in use because it has been automatically connected to another device
> (did you need 'if=none' in the drive options?)
> The command line will be changed to include 'if=none', as the
will have to be changed
> error message suggests.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/arm/virt.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index f1e85c8..7e643ba 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -966,6 +966,8 @@ static void virt_class_init(ObjectClass *oc, void *data)
> mc->init = machvirt_init;
> mc->max_cpus = 8;
> mc->has_dynamic_sysbus = true;
> + mc->block_default_type = IF_VIRTIO;
> + mc->no_cdrom = 1;
> }
>
> static const TypeInfo machvirt_info = {
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.4 3/3] hw/arm/virt: Make block devices default to virtio
2015-06-25 7:40 ` Markus Armbruster
@ 2015-06-25 9:04 ` Peter Maydell
2015-06-25 9:25 ` Markus Armbruster
0 siblings, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2015-06-25 9:04 UTC (permalink / raw
To: Markus Armbruster; +Cc: Kevin Wolf, QEMU Developers, qemu-block, Patch Tracking
On 25 June 2015 at 08:40, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> Now we have virtio-pci, we can make the virt board's default block
>> device type be IF_VIRTIO. This allows users to use simplified
>> command lines that don't have to explicitly create virtio-pci-blk
>> devices; the -hda &c very short options now also work.
>>
>> This means we also need to set no_cdrom to avoid getting a
>> default cdrom device -- this is needed because the virtio-blk
>> device will fail if it is connected to a block backend with
>> no media, which is what the default cdrom device typically is.
>> Providing a cdrom with media via -cdrom will still work.
>
> It'll create a virtio-blk device with non-removable medium, won't it?
Yes, I think so. Mostly I cared that -cdrom won't make qemu die with a
confusing error.
(Without no_cdrom, qemu dies even if you don't say -cdrom, because
of the default empty drive.)
>> The command line will be changed to include 'if=none', as the
>
> will have to be changed
Yes.
-- PMM
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.4 3/3] hw/arm/virt: Make block devices default to virtio
2015-06-25 9:04 ` Peter Maydell
@ 2015-06-25 9:25 ` Markus Armbruster
0 siblings, 0 replies; 12+ messages in thread
From: Markus Armbruster @ 2015-06-25 9:25 UTC (permalink / raw
To: Peter Maydell; +Cc: Kevin Wolf, QEMU Developers, qemu-block, Patch Tracking
Peter Maydell <peter.maydell@linaro.org> writes:
> On 25 June 2015 at 08:40, Markus Armbruster <armbru@redhat.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>>> Now we have virtio-pci, we can make the virt board's default block
>>> device type be IF_VIRTIO. This allows users to use simplified
>>> command lines that don't have to explicitly create virtio-pci-blk
>>> devices; the -hda &c very short options now also work.
>>>
>>> This means we also need to set no_cdrom to avoid getting a
>>> default cdrom device -- this is needed because the virtio-blk
>>> device will fail if it is connected to a block backend with
>>> no media, which is what the default cdrom device typically is.
>>> Providing a cdrom with media via -cdrom will still work.
>>
>> It'll create a virtio-blk device with non-removable medium, won't it?
>
> Yes, I think so. Mostly I cared that -cdrom won't make qemu die with a
> confusing error.
>
> (Without no_cdrom, qemu dies even if you don't say -cdrom, because
> of the default empty drive.)
Instead of "will still work", I'd like to see something like "will
succeed, but silently create a device with non-removable medium, which
is probably not what you want, but the best we can do now".
>>> The command line will be changed to include 'if=none', as the
>>
>> will have to be changed
>
> Yes.
>
> -- PMM
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.4 0/3] block: Improve warnings for doubly-connected drives
2015-06-23 14:01 [Qemu-devel] [PATCH v2 for-2.4 0/3] block: Improve warnings for doubly-connected drives Peter Maydell
` (2 preceding siblings ...)
2015-06-23 14:01 ` [Qemu-devel] [PATCH v2 for-2.4 3/3] hw/arm/virt: Make block devices default to virtio Peter Maydell
@ 2015-06-25 9:26 ` Markus Armbruster
2015-06-25 9:35 ` Peter Maydell
2015-06-25 13:23 ` [Qemu-devel] " Stefan Hajnoczi
4 siblings, 1 reply; 12+ messages in thread
From: Markus Armbruster @ 2015-06-25 9:26 UTC (permalink / raw
To: Peter Maydell; +Cc: Kevin Wolf, qemu-devel, qemu-block, patches
Peter Maydell <peter.maydell@linaro.org> writes:
> This patchset attempts to improve the warning and error messages for
> bad user command lines that attempt to connect a drive up to two
> devices. The motivation here is patch #4, which changes the default
> interface for the virt board to virtio. That will break some existing
> command lines which forgot to specify if=none, and so I would like
> us to at least diagnose that user error in a helpful way that points
> the user towards adding the missing if=none.
>
> Version 2 reduces scope, because it turns out that it is harder than
> we thought to identify whether a use of an if=something drive is the
> auto-plugging or a manual wiring up to a device. So all we do here
> is improve the error messages for two situations which were already
> errors but with rather cryptic messages:
>
> (1) Drive specified as to be auto-connected and also manually connected
> (and the board does handle this if= type):
>
> qemu-system-x86_64 -nodefaults -display none \
> -drive if=scsi,file=tmp.qcow2,id=foo -device ide-hd,drive=foo
>
> Previously:
> qemu-system-x86_64: -device ide-hd,drive=foo: Property 'ide-hd.drive'
> can't take value 'foo', it's in use
>
> Now:
> qemu-system-x86_64: -device ide-hd,drive=foo: Drive 'foo' is already in
> use because it has been automatically connected to another device (did
> you need 'if=none' in the drive options?)
>
> (2) Drive specified to be manually connected in two different ways:
>
> qemu-system-x86_64 -nodefaults -display none \
> -drive if=none,file=tmp.qcow2,id=foo -device ide-hd,drive=foo \
> -device ide-hd,drive=foo
>
> Previously:
> qemu-system-x86_64: -device ide-hd,drive=foo: Property 'ide-hd.drive'
> can't take value 'foo', it's in use
>
> Now:
> qemu-system-x86_64: -device ide-hd,drive=foo: Drive 'foo' is already in
> use by another device
>
> (We'll also produce message 1 in the oddball case where the user creates
> a drive if=something-not-handled-by-machine and then wires it up manually
> to two different devices; in this case their command line is doubly broken
> and if they use if=none as suggested by message 1 they'll then get message 2
> and can fix their own double-usage...)
With the commit message of PATCH 3 amended, series
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.4 0/3] block: Improve warnings for doubly-connected drives
2015-06-25 9:26 ` [Qemu-devel] [PATCH v2 for-2.4 0/3] block: Improve warnings for doubly-connected drives Markus Armbruster
@ 2015-06-25 9:35 ` Peter Maydell
2015-06-25 10:49 ` Markus Armbruster
0 siblings, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2015-06-25 9:35 UTC (permalink / raw
To: Markus Armbruster; +Cc: Kevin Wolf, QEMU Developers, qemu-block, Patch Tracking
On 25 June 2015 at 10:26, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> This patchset attempts to improve the warning and error messages for
>> bad user command lines that attempt to connect a drive up to two
>> devices. The motivation here is patch #4, which changes the default
>> interface for the virt board to virtio. That will break some existing
>> command lines which forgot to specify if=none, and so I would like
>> us to at least diagnose that user error in a helpful way that points
>> the user towards adding the missing if=none.
> With the commit message of PATCH 3 amended, series
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
Thanks. Do you want to take this through a block tree or are you happy
for me to put it through target-arm.next?
-- PMM
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.4 0/3] block: Improve warnings for doubly-connected drives
2015-06-25 9:35 ` Peter Maydell
@ 2015-06-25 10:49 ` Markus Armbruster
2015-06-25 13:23 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
0 siblings, 1 reply; 12+ messages in thread
From: Markus Armbruster @ 2015-06-25 10:49 UTC (permalink / raw
To: Peter Maydell
Cc: Kevin Wolf, Stefan Hajnoczi, QEMU Developers, qemu-block,
Patch Tracking
Peter Maydell <peter.maydell@linaro.org> writes:
> On 25 June 2015 at 10:26, Markus Armbruster <armbru@redhat.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>>> This patchset attempts to improve the warning and error messages for
>>> bad user command lines that attempt to connect a drive up to two
>>> devices. The motivation here is patch #4, which changes the default
>>> interface for the virt board to virtio. That will break some existing
>>> command lines which forgot to specify if=none, and so I would like
>>> us to at least diagnose that user error in a helpful way that points
>>> the user towards adding the missing if=none.
>
>> With the commit message of PATCH 3 amended, series
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
> Thanks. Do you want to take this through a block tree or are you happy
> for me to put it through target-arm.next?
I'm fine with target-arm. Copying Stefan in the unlikely case he's not.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.4 0/3] block: Improve warnings for doubly-connected drives
2015-06-23 14:01 [Qemu-devel] [PATCH v2 for-2.4 0/3] block: Improve warnings for doubly-connected drives Peter Maydell
` (3 preceding siblings ...)
2015-06-25 9:26 ` [Qemu-devel] [PATCH v2 for-2.4 0/3] block: Improve warnings for doubly-connected drives Markus Armbruster
@ 2015-06-25 13:23 ` Stefan Hajnoczi
4 siblings, 0 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2015-06-25 13:23 UTC (permalink / raw
To: Peter Maydell
Cc: Kevin Wolf, patches, qemu-devel, qemu-block, Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 3111 bytes --]
On Tue, Jun 23, 2015 at 03:01:44PM +0100, Peter Maydell wrote:
> This patchset attempts to improve the warning and error messages for
> bad user command lines that attempt to connect a drive up to two
> devices. The motivation here is patch #4, which changes the default
> interface for the virt board to virtio. That will break some existing
> command lines which forgot to specify if=none, and so I would like
> us to at least diagnose that user error in a helpful way that points
> the user towards adding the missing if=none.
>
> Version 2 reduces scope, because it turns out that it is harder than
> we thought to identify whether a use of an if=something drive is the
> auto-plugging or a manual wiring up to a device. So all we do here
> is improve the error messages for two situations which were already
> errors but with rather cryptic messages:
>
> (1) Drive specified as to be auto-connected and also manually connected
> (and the board does handle this if= type):
>
> qemu-system-x86_64 -nodefaults -display none \
> -drive if=scsi,file=tmp.qcow2,id=foo -device ide-hd,drive=foo
>
> Previously:
> qemu-system-x86_64: -device ide-hd,drive=foo: Property 'ide-hd.drive'
> can't take value 'foo', it's in use
>
> Now:
> qemu-system-x86_64: -device ide-hd,drive=foo: Drive 'foo' is already in
> use because it has been automatically connected to another device (did
> you need 'if=none' in the drive options?)
>
> (2) Drive specified to be manually connected in two different ways:
>
> qemu-system-x86_64 -nodefaults -display none \
> -drive if=none,file=tmp.qcow2,id=foo -device ide-hd,drive=foo \
> -device ide-hd,drive=foo
>
> Previously:
> qemu-system-x86_64: -device ide-hd,drive=foo: Property 'ide-hd.drive'
> can't take value 'foo', it's in use
>
> Now:
> qemu-system-x86_64: -device ide-hd,drive=foo: Drive 'foo' is already in
> use by another device
>
> (We'll also produce message 1 in the oddball case where the user creates
> a drive if=something-not-handled-by-machine and then wires it up manually
> to two different devices; in this case their command line is doubly broken
> and if they use if=none as suggested by message 1 they'll then get message 2
> and can fix their own double-usage...)
>
> Changes v1->v2:
> * drop "warn if an if=<something> drive was also connected manually" patch
> * change implementation of "improve error message" patch
>
>
> Peter Maydell (3):
> qdev-properties-system: Change set_pointer's parse callback to use
> Error
> qdev-properties-system: Improve error message for drive assignment
> conflict
> hw/arm/virt: Make block devices default to virtio
>
> hw/arm/virt.c | 2 ++
> hw/core/qdev-properties-system.c | 42 +++++++++++++++++++++++++++-------------
> 2 files changed, 31 insertions(+), 13 deletions(-)
>
> --
> 1.9.1
>
>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v2 for-2.4 0/3] block: Improve warnings for doubly-connected drives
2015-06-25 10:49 ` Markus Armbruster
@ 2015-06-25 13:23 ` Stefan Hajnoczi
0 siblings, 0 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2015-06-25 13:23 UTC (permalink / raw
To: Markus Armbruster
Cc: Kevin Wolf, Peter Maydell, qemu-block, Patch Tracking,
QEMU Developers, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 1168 bytes --]
On Thu, Jun 25, 2015 at 12:49:31PM +0200, Markus Armbruster wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On 25 June 2015 at 10:26, Markus Armbruster <armbru@redhat.com> wrote:
> >> Peter Maydell <peter.maydell@linaro.org> writes:
> >>
> >>> This patchset attempts to improve the warning and error messages for
> >>> bad user command lines that attempt to connect a drive up to two
> >>> devices. The motivation here is patch #4, which changes the default
> >>> interface for the virt board to virtio. That will break some existing
> >>> command lines which forgot to specify if=none, and so I would like
> >>> us to at least diagnose that user error in a helpful way that points
> >>> the user towards adding the missing if=none.
> >
> >> With the commit message of PATCH 3 amended, series
> >> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> >
> > Thanks. Do you want to take this through a block tree or are you happy
> > for me to put it through target-arm.next?
>
> I'm fine with target-arm. Copying Stefan in the unlikely case he's not.
Thanks, I'm happy with this series. Please take it through the arm
tree.
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-06-25 13:23 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-23 14:01 [Qemu-devel] [PATCH v2 for-2.4 0/3] block: Improve warnings for doubly-connected drives Peter Maydell
2015-06-23 14:01 ` [Qemu-devel] [PATCH v2 for-2.4 1/3] qdev-properties-system: Change set_pointer's parse callback to use Error Peter Maydell
2015-06-23 14:01 ` [Qemu-devel] [PATCH v2 for-2.4 2/3] qdev-properties-system: Improve error message for drive assignment conflict Peter Maydell
2015-06-23 14:01 ` [Qemu-devel] [PATCH v2 for-2.4 3/3] hw/arm/virt: Make block devices default to virtio Peter Maydell
2015-06-25 7:40 ` Markus Armbruster
2015-06-25 9:04 ` Peter Maydell
2015-06-25 9:25 ` Markus Armbruster
2015-06-25 9:26 ` [Qemu-devel] [PATCH v2 for-2.4 0/3] block: Improve warnings for doubly-connected drives Markus Armbruster
2015-06-25 9:35 ` Peter Maydell
2015-06-25 10:49 ` Markus Armbruster
2015-06-25 13:23 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2015-06-25 13:23 ` [Qemu-devel] " Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.