From: Anthony Liguori <aliguori@redhat.com>
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:33 -0600 [thread overview]
Message-ID: <4B966309.1010107@redhat.com> (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 [this message]
2010-03-09 15:02 ` Anthony Liguori
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=4B966309.1010107@redhat.com \
--to=aliguori@redhat.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).