From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Np0xP-0007Cb-Ks for qemu-devel@nongnu.org; Tue, 09 Mar 2010 10:02:51 -0500 Received: from [199.232.76.173] (port=54942 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Np0xO-0007BV-UE for qemu-devel@nongnu.org; Tue, 09 Mar 2010 10:02:50 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1Np0xM-0001rp-Rf for qemu-devel@nongnu.org; Tue, 09 Mar 2010 10:02:50 -0500 Received: from ey-out-1920.google.com ([74.125.78.148]:39734) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Np0xM-0001rj-DT for qemu-devel@nongnu.org; Tue, 09 Mar 2010 10:02:48 -0500 Received: by ey-out-1920.google.com with SMTP id 5so3967181eyb.14 for ; Tue, 09 Mar 2010 07:02:47 -0800 (PST) Message-ID: <4B96630E.7040403@codemonkey.ws> Date: Tue, 09 Mar 2010 09:02:38 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH v2 1/2] qdev: Improve diagnostics for bad property values References: <1267195851-22244-1-git-send-email-armbru@redhat.com> <1267195851-22244-2-git-send-email-armbru@redhat.com> In-Reply-To: <1267195851-22244-2-git-send-email-armbru@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Mark McLoughlin , "Michael S. Tsirkin" , qemu-devel@nongnu.org, Gerd Hoffmann 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 > 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; >