From: Anthony Liguori <anthony@codemonkey.ws>
To: Markus Armbruster <armbru@redhat.com>
Cc: Mark McLoughlin <markmc@redhat.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
qemu-devel@nongnu.org, Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 1/2] qdev: Improve diagnostics for bad property values
Date: Tue, 09 Mar 2010 09:02:38 -0600 [thread overview]
Message-ID: <4B96630E.7040403@codemonkey.ws> (raw)
In-Reply-To: <1267195851-22244-2-git-send-email-armbru@redhat.com>
On 02/26/2010 08:50 AM, Markus Armbruster wrote:
> Property "vlan" reports "failed to parse" even when the value parses
> just fine, but the result doesn't name an existing VLAN.
>
> Similarly, properties "drive", "chr" and "netdev" misleadingly report
> "failed to parse" when the value doesn't name an existing host device.
>
> Change PropertyInfo method parse to return an error code, so that
> qdev_prop_parse() can report the error more accurately.
>
> Signed-off-by: Markus Armbruster<armbru@redhat.com>
>
Applied all. Thanks.
Regards,
Anthony Liguori
> ---
> hw/qdev-properties.c | 57 +++++++++++++++++++++++++++++--------------------
> 1 files changed, 34 insertions(+), 23 deletions(-)
>
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 277ff9e..438eaea 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -44,7 +44,7 @@ static int parse_bit(DeviceState *dev, Property *prop, const char *str)
> else if (!strncasecmp(str, "off", 3))
> bit_prop_set(dev, prop, false);
> else
> - return -1;
> + return -EINVAL;
> return 0;
> }
>
> @@ -72,7 +72,7 @@ static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
> /* accept both hex and decimal */
> fmt = strncasecmp(str, "0x",2) == 0 ? "%" PRIx8 : "%" PRIu8;
> if (sscanf(str, fmt, ptr) != 1)
> - return -1;
> + return -EINVAL;
> return 0;
> }
>
> @@ -100,7 +100,7 @@ static int parse_uint16(DeviceState *dev, Property *prop, const char *str)
> /* accept both hex and decimal */
> fmt = strncasecmp(str, "0x",2) == 0 ? "%" PRIx16 : "%" PRIu16;
> if (sscanf(str, fmt, ptr) != 1)
> - return -1;
> + return -EINVAL;
> return 0;
> }
>
> @@ -128,7 +128,7 @@ static int parse_uint32(DeviceState *dev, Property *prop, const char *str)
> /* accept both hex and decimal */
> fmt = strncasecmp(str, "0x",2) == 0 ? "%" PRIx32 : "%" PRIu32;
> if (sscanf(str, fmt, ptr) != 1)
> - return -1;
> + return -EINVAL;
> return 0;
> }
>
> @@ -151,7 +151,7 @@ static int parse_int32(DeviceState *dev, Property *prop, const char *str)
> int32_t *ptr = qdev_get_prop_ptr(dev, prop);
>
> if (sscanf(str, "%" PRId32, ptr) != 1)
> - return -1;
> + return -EINVAL;
> return 0;
> }
>
> @@ -176,7 +176,7 @@ static int parse_hex32(DeviceState *dev, Property *prop, const char *str)
> uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
>
> if (sscanf(str, "%" PRIx32, ptr) != 1)
> - return -1;
> + return -EINVAL;
> return 0;
> }
>
> @@ -204,7 +204,7 @@ static int parse_uint64(DeviceState *dev, Property *prop, const char *str)
> /* accept both hex and decimal */
> fmt = strncasecmp(str, "0x",2) == 0 ? "%" PRIx64 : "%" PRIu64;
> if (sscanf(str, fmt, ptr) != 1)
> - return -1;
> + return -EINVAL;
> return 0;
> }
>
> @@ -229,7 +229,7 @@ static int parse_hex64(DeviceState *dev, Property *prop, const char *str)
> uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
>
> if (sscanf(str, "%" PRIx64, ptr) != 1)
> - return -1;
> + return -EINVAL;
> return 0;
> }
>
> @@ -283,7 +283,7 @@ static int parse_drive(DeviceState *dev, Property *prop, const char *str)
>
> *ptr = drive_get_by_id(str);
> if (*ptr == NULL)
> - return -1;
> + return -ENOENT;
> return 0;
> }
>
> @@ -309,7 +309,7 @@ static int parse_chr(DeviceState *dev, Property *prop, const char *str)
>
> *ptr = qemu_chr_find(str);
> if (*ptr == NULL)
> - return -1;
> + return -ENOENT;
> return 0;
> }
>
> @@ -340,7 +340,7 @@ static int parse_netdev(DeviceState *dev, Property *prop, const char *str)
>
> *ptr = qemu_find_netdev(str);
> if (*ptr == NULL)
> - return -1;
> + return -ENOENT;
> return 0;
> }
>
> @@ -371,10 +371,10 @@ static int parse_vlan(DeviceState *dev, Property *prop, const char *str)
> int id;
>
> if (sscanf(str, "%d",&id) != 1)
> - return -1;
> + return -EINVAL;
> *ptr = qemu_find_vlan(id, 1);
> if (*ptr == NULL)
> - return -1;
> + return -ENOENT;
> return 0;
> }
>
> @@ -427,15 +427,15 @@ static int parse_mac(DeviceState *dev, Property *prop, const char *str)
>
> for (i = 0, pos = 0; i< 6; i++, pos += 3) {
> if (!qemu_isxdigit(str[pos]))
> - return -1;
> + return -EINVAL;
> if (!qemu_isxdigit(str[pos+1]))
> - return -1;
> + return -EINVAL;
> if (i == 5) {
> if (str[pos+2] != '\0')
> - return -1;
> + return -EINVAL;
> } else {
> if (str[pos+2] != ':'&& str[pos+2] != '-')
> - return -1;
> + return -EINVAL;
> }
> mac->a[i] = strtol(str+pos,&p, 16);
> }
> @@ -472,13 +472,13 @@ static int parse_pci_devfn(DeviceState *dev, Property *prop, const char *str)
> if (sscanf(str, "%x.%x%n",&slot,&fn,&n) != 2) {
> fn = 0;
> if (sscanf(str, "%x%n",&slot,&n) != 1) {
> - return -1;
> + return -EINVAL;
> }
> }
> if (str[n] != '\0')
> - return -1;
> + return -EINVAL;
> if (fn> 7)
> - return -1;
> + return -EINVAL;
> *ptr = slot<< 3 | fn;
> return 0;
> }
> @@ -541,6 +541,7 @@ int qdev_prop_exists(DeviceState *dev, const char *name)
> int qdev_prop_parse(DeviceState *dev, const char *name, const char *value)
> {
> Property *prop;
> + int ret;
>
> prop = qdev_prop_find(dev, name);
> if (!prop) {
> @@ -553,9 +554,19 @@ int qdev_prop_parse(DeviceState *dev, const char *name, const char *value)
> dev->info->name, name);
> return -1;
> }
> - if (prop->info->parse(dev, prop, value) != 0) {
> - fprintf(stderr, "property \"%s.%s\": failed to parse \"%s\"\n",
> - dev->info->name, name, value);
> + ret = prop->info->parse(dev, prop, value);
> + if (ret< 0) {
> + switch (ret) {
> + default:
> + case -EINVAL:
> + fprintf(stderr, "property \"%s.%s\": failed to parse \"%s\"\n",
> + dev->info->name, name, value);
> + break;
> + case -ENOENT:
> + fprintf(stderr, "property \"%s.%s\": could not find \"%s\"\n",
> + dev->info->name, name, value);
> + break;
> + }
> return -1;
> }
> return 0;
>
next prev parent reply other threads:[~2010-03-09 15:02 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-02-26 14:50 [Qemu-devel] [PATCH v2 0/2] Catch attempt to attach more than one device to a netdev Markus Armbruster
2010-02-26 14:50 ` [Qemu-devel] [PATCH v2 1/2] qdev: Improve diagnostics for bad property values Markus Armbruster
2010-03-09 15:02 ` Anthony Liguori
2010-03-09 15:02 ` Anthony Liguori [this message]
2010-02-26 14:50 ` [Qemu-devel] [PATCH v2 2/2] qdev: Catch attempt to attach more than one device to a netdev Markus Armbruster
2010-02-26 14:52 ` [Qemu-devel] Re: [PATCH v2 0/2] " Michael S. Tsirkin
2010-02-26 16:00 ` Gerd Hoffmann
2010-02-27 9:40 ` Juan Quintela
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4B96630E.7040403@codemonkey.ws \
--to=anthony@codemonkey.ws \
--cc=armbru@redhat.com \
--cc=kraxel@redhat.com \
--cc=markmc@redhat.com \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.