All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/2] Catch attempt to attach more than one device to a netdev
@ 2010-02-26 14:50 Markus Armbruster
  2010-02-26 14:50 ` [Qemu-devel] [PATCH v2 1/2] qdev: Improve diagnostics for bad property values Markus Armbruster
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Markus Armbruster @ 2010-02-26 14:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin, Gerd Hoffmann, Michael S. Tsirkin

Markus Armbruster (2):
  qdev: Improve diagnostics for bad property values
  qdev: Catch attempt to attach more than one device to a netdev

 hw/qdev-properties.c |   64 ++++++++++++++++++++++++++++++++------------------
 net.c                |    1 +
 2 files changed, 42 insertions(+), 23 deletions(-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH v2 1/2] qdev: Improve diagnostics for bad property values
  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 ` Markus Armbruster
  2010-03-09 15:02   ` Anthony Liguori
  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
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 8+ messages in thread
From: Markus Armbruster @ 2010-02-26 14:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin, Gerd Hoffmann, Michael S. Tsirkin

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>
---
 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;
-- 
1.6.6.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH v2 2/2] qdev: Catch attempt to attach more than one device to a netdev
  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-02-26 14:50 ` Markus Armbruster
  2010-02-26 14:52 ` [Qemu-devel] Re: [PATCH v2 0/2] " Michael S. Tsirkin
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2010-02-26 14:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark McLoughlin, Gerd Hoffmann, Michael S. Tsirkin

Guest device and host netdev are peers, i.e. it's a 1:1 relation.
However, we fail to enforce that:

    $ qemu -nodefaults --nographic -netdev user,id=net0 -device e1000,netdev=net0 -device virtio-net-pci,netdev=net0 -monitor stdio
    QEMU 0.12.50 monitor - type 'help' for more information
    (qemu) info network
    Devices not on any VLAN:
      net0: net=10.0.2.0, restricted=n peer=virtio-net-pci.0
      e1000.0: model=e1000,macaddr=52:54:00:12:34:56 peer=net0
      virtio-net-pci.0: model=virtio-net-pci,macaddr=52:54:00:12:34:57 peer=net0

It's all downhill from there.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/qdev-properties.c |    7 +++++++
 net.c                |    1 +
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 438eaea..24671af 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -341,6 +341,9 @@ static int parse_netdev(DeviceState *dev, Property *prop, const char *str)
     *ptr = qemu_find_netdev(str);
     if (*ptr == NULL)
         return -ENOENT;
+    if ((*ptr)->peer) {
+        return -EEXIST;
+    }
     return 0;
 }
 
@@ -557,6 +560,10 @@ int qdev_prop_parse(DeviceState *dev, const char *name, const char *value)
     ret = prop->info->parse(dev, prop, value);
     if (ret < 0) {
         switch (ret) {
+        case -EEXIST:
+            fprintf(stderr, "property \"%s.%s\": \"%s\" is already in use\n",
+                    dev->info->name, name, value);
+            break;
         default:
         case -EINVAL:
             fprintf(stderr, "property \"%s.%s\": failed to parse \"%s\"\n",
diff --git a/net.c b/net.c
index a1bf49f..e6c96d3 100644
--- a/net.c
+++ b/net.c
@@ -245,6 +245,7 @@ VLANClientState *qemu_new_net_client(NetClientInfo *info,
         QTAILQ_INSERT_TAIL(&vc->vlan->clients, vc, next);
     } else {
         if (peer) {
+            assert(!peer->peer);
             vc->peer = peer;
             peer->peer = vc;
         }
-- 
1.6.6.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] Re: [PATCH v2 0/2] Catch attempt to attach more than one device to a netdev
  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-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 ` Michael S. Tsirkin
  2010-02-26 16:00 ` Gerd Hoffmann
  2010-02-27  9:40 ` Juan Quintela
  4 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2010-02-26 14:52 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Mark McLoughlin, qemu-devel, Gerd Hoffmann

On Fri, Feb 26, 2010 at 03:50:49PM +0100, Markus Armbruster wrote:
> Markus Armbruster (2):
>   qdev: Improve diagnostics for bad property values
>   qdev: Catch attempt to attach more than one device to a netdev
> 
>  hw/qdev-properties.c |   64 ++++++++++++++++++++++++++++++++------------------
>  net.c                |    1 +
>  2 files changed, 42 insertions(+), 23 deletions(-)

Acked-by: Michael S. Tsirkin <mst@redhat.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] Re: [PATCH v2 0/2] Catch attempt to attach more than one device to a netdev
  2010-02-26 14:50 [Qemu-devel] [PATCH v2 0/2] Catch attempt to attach more than one device to a netdev Markus Armbruster
                   ` (2 preceding siblings ...)
  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
  4 siblings, 0 replies; 8+ messages in thread
From: Gerd Hoffmann @ 2010-02-26 16:00 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Mark McLoughlin, qemu-devel, Michael S. Tsirkin

On 02/26/10 15:50, Markus Armbruster wrote:
> Markus Armbruster (2):
>    qdev: Improve diagnostics for bad property values
>    qdev: Catch attempt to attach more than one device to a netdev

Looks good.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

cheers,
   Gerd

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] Re: [PATCH v2 0/2] Catch attempt to attach more than one device to a netdev
  2010-02-26 14:50 [Qemu-devel] [PATCH v2 0/2] Catch attempt to attach more than one device to a netdev Markus Armbruster
                   ` (3 preceding siblings ...)
  2010-02-26 16:00 ` Gerd Hoffmann
@ 2010-02-27  9:40 ` Juan Quintela
  4 siblings, 0 replies; 8+ messages in thread
From: Juan Quintela @ 2010-02-27  9:40 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Mark McLoughlin, Michael S. Tsirkin, qemu-devel, Gerd Hoffmann

Markus Armbruster <armbru@redhat.com> wrote:
> Markus Armbruster (2):
>   qdev: Improve diagnostics for bad property values
>   qdev: Catch attempt to attach more than one device to a netdev
>
>  hw/qdev-properties.c |   64 ++++++++++++++++++++++++++++++++------------------
>  net.c                |    1 +
>  2 files changed, 42 insertions(+), 23 deletions(-)

Acked-by: Juan Quintela <quintela@redhat.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/2] qdev: Improve diagnostics for bad property values
  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
  1 sibling, 0 replies; 8+ messages in thread
From: Anthony Liguori @ 2010-03-09 15:02 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Mark McLoughlin, Michael S. Tsirkin, qemu-devel, 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<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;
>    

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/2] qdev: Improve diagnostics for bad property values
  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
  1 sibling, 0 replies; 8+ messages in thread
From: Anthony Liguori @ 2010-03-09 15:02 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Mark McLoughlin, Michael S. Tsirkin, qemu-devel, 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<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;
>    

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2010-03-09 15:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

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.