[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/3] conf: Rework virDomainDeviceDefParse



On 07/11/13 13:29, Michal Privoznik wrote:
When adding a new domain device, it is fairly easy to forget to add
corresponding piece into virDomainDeviceDefParse. However, if the
internal structure is changed to one bit switch() the compiler will warn
about not handled enum item.
---
  src/conf/domain_conf.c | 96 ++++++++++++++++++++++++++++----------------------
  1 file changed, 54 insertions(+), 42 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d0c87b2..764ca6e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9336,94 +9336,106 @@ virDomainDeviceDefParse(const char *xmlStr,
      if (VIR_ALLOC(dev) < 0)
          goto error;

-    if (xmlStrEqual(node->name, BAD_CAST "disk")) {
-        dev->type = VIR_DOMAIN_DEVICE_DISK;
+    if ((dev->type = virDomainDeviceTypeFromString((const char *) node->name)) < 0) {
+        /* Some crazy mapping of serial, parallel, console and channel to
+         * VIR_DOMAIN_DEVICE_CHR. */

hmmm, this isn't very nice, but ...

+        if (xmlStrEqual(node->name, BAD_CAST "channel") ||
+            xmlStrEqual(node->name, BAD_CAST "console") ||
+            xmlStrEqual(node->name, BAD_CAST "parallel") ||
+            xmlStrEqual(node->name, BAD_CAST "serial")) {
+            dev->type = VIR_DOMAIN_DEVICE_CHR;
+        } else {
+            virReportError(VIR_ERR_XML_ERROR, _("unknown device type '%s'"), node->name);

This line is probably too long. Please break it.

+            goto error;
+        }
+    }
+
+    switch ((virDomainDeviceType) dev->type) {
+    case VIR_DOMAIN_DEVICE_DISK:
          if (!(dev->data.disk = virDomainDiskDefParseXML(xmlopt, node, ctxt,
                                                          NULL, def->seclabels,
                                                          def->nseclabels,
                                                          flags)))
              goto error;
-    } else if (xmlStrEqual(node->name, BAD_CAST "lease")) {
-        dev->type = VIR_DOMAIN_DEVICE_LEASE;
+        break;
+    case VIR_DOMAIN_DEVICE_LEASE:
          if (!(dev->data.lease = virDomainLeaseDefParseXML(node)))
              goto error;
-    } else if (xmlStrEqual(node->name, BAD_CAST "filesystem")) {
-        dev->type = VIR_DOMAIN_DEVICE_FS;
+        break;

... this pattern looks better and as the commit message states, it lets the compiler warn us.

ACK, with long line broken.

Peter


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]