[libvirt] [PATCH 17/18] conf: use disk source accessors in xenxs/

Wednesday, 19 March 2014

Part of a series of cleanups to use new accessor methods.

* src/xenxs/xen_sxpr.c (xenParseSxprDisks, xenParseSxpr)
(xenFormatSxprDisk, xenFormatSxpr): Use accessors.
* src/xenxs/xen_xm.c (xenParseXM, xenFormatXMDisk, xenFormatXM):
Likewise.

Signed-off-by: Eric Blake <eblake(a)redhat.com&gt;
---

This one is a bit trickier to review, in that it is replacing
in-place strndup over to calls to a function that mallocs
a copy of the input string.  While it compiles and passes
'make check', I don't actually have a xen setup to actually
prove that it works.

 src/xenxs/xen_sxpr.c | 111 ++++++++++++++++++++++++++------------------------
 src/xenxs/xen_xm.c   | 113 +++++++++++++++++++++++++++------------------------
 2 files changed, 118 insertions(+), 106 deletions(-)

diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index 01d1ca1..e03e254 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -1,7 +1,7 @@
 /*
  * xen_sxpr.c: Xen SEXPR parsing functions
  *
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 Red Hat, Inc.
  * Copyright (C) 2011 Univention GmbH
  * Copyright (C) 2005 Anthony Liguori <aliguori(a)us.ibm.com&gt;
  *
@@ -401,24 +401,23 @@ xenParseSxprDisks(virDomainDefPtr def,

                 if (sexpr_lookup(node, "device/tap2") &&
                     STRPREFIX(src, "tap:")) {
-                    if (VIR_STRDUP(disk->driverName, "tap2") < 0)
+                    if (virDomainDiskSetDriver(disk, "tap2") < 0)
                         goto error;
                 } else {
-                    if (VIR_ALLOC_N(disk->driverName, (offset-src)+1) < 0)
+                    char *tmp;
+                    if (VIR_STRNDUP(tmp, src, offset - src) < 0)
                         goto error;
-                    if (virStrncpy(disk->driverName, src, offset-src,
-                                   (offset-src)+1) == NULL) {
-                        virReportError(VIR_ERR_INTERNAL_ERROR,
-                                       _("Driver name %s too big for
destination"),
-                                       src);
+                    if (virDomainDiskSetDriver(disk, tmp) < 0) {
+                        VIR_FREE(tmp);
                         goto error;
                     }
+                    VIR_FREE(tmp);
                 }

                 src = offset + 1;

-                if (STREQ(disk->driverName, "tap") ||
-                    STREQ(disk->driverName, "tap2")) {
+                if (STREQ(virDomainDiskGetDriver(disk), "tap") ||
+                    STREQ(virDomainDiskGetDriver(disk), "tap2")) {
                     char *driverType = NULL;

                     offset = strchr(src, ':');
@@ -431,12 +430,12 @@ xenParseSxprDisks(virDomainDefPtr def,
                     if (VIR_STRNDUP(driverType, src, offset - src) < 0)
                         goto error;
                     if (STREQ(driverType, "aio"))
-                        disk->format = VIR_STORAGE_FILE_RAW;
+                        virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
                     else
-                        disk->format =
-                            virStorageFileFormatTypeFromString(driverType);
+                        virDomainDiskSetFormat(disk,
+                                              
virStorageFileFormatTypeFromString(driverType));
                     VIR_FREE(driverType);
-                    if (disk->format <= 0) {
+                    if (virDomainDiskGetFormat(disk) <= 0) {
                         virReportError(VIR_ERR_INTERNAL_ERROR,
                                        _("Unknown driver type %s"), src);
                         goto error;
@@ -448,17 +447,17 @@ xenParseSxprDisks(virDomainDefPtr def,
                        so we assume common case here. If blktap becomes
                        omnipotent, we can revisit this, perhaps stat()'ing
                        the src file in question */
-                    disk->type = VIR_DOMAIN_DISK_TYPE_FILE;
-                } else if (STREQ(disk->driverName, "phy")) {
-                    disk->type = VIR_DOMAIN_DISK_TYPE_BLOCK;
-                } else if (STREQ(disk->driverName, "file")) {
-                    disk->type = VIR_DOMAIN_DISK_TYPE_FILE;
+                    virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE);
+                } else if (STREQ(virDomainDiskGetDriver(disk), "phy")) {
+                    virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_BLOCK);
+                } else if (STREQ(virDomainDiskGetDriver(disk), "file")) {
+                    virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE);
                 }
             } else {
                 /* No CDROM media so can't really tell. We'll just
                    call if a FILE for now and update when media
                    is inserted later */
-                disk->type = VIR_DOMAIN_DISK_TYPE_FILE;
+                virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE);
             }

             if (STREQLEN(dst, "ioemu:", 6))
@@ -482,7 +481,7 @@ xenParseSxprDisks(virDomainDefPtr def,

             if (VIR_STRDUP(disk->dst, dst) < 0)
                 goto error;
-            if (VIR_STRDUP(disk->src, src) < 0)
+            if (virDomainDiskSetSource(disk, src) < 0)
                 goto error;

             if (STRPREFIX(disk->dst, "xvd"))
@@ -1307,17 +1306,17 @@ xenParseSxpr(const struct sexpr *root,
             virDomainDiskDefPtr disk;
             if (VIR_ALLOC(disk) < 0)
                 goto error;
-            if (VIR_STRDUP(disk->src, tmp) < 0) {
+            if (virDomainDiskSetSource(disk, tmp) < 0) {
                 virDomainDiskDefFree(disk);
                 goto error;
             }
-            disk->type = VIR_DOMAIN_DISK_TYPE_FILE;
+            virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE);
             disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
             if (VIR_STRDUP(disk->dst, "hdc") < 0) {
                 virDomainDiskDefFree(disk);
                 goto error;
             }
-            if (VIR_STRDUP(disk->driverName, "file") < 0) {
+            if (virDomainDiskSetDriver(disk, "file") < 0) {
                 virDomainDiskDefFree(disk);
                 goto error;
             }
@@ -1342,17 +1341,17 @@ xenParseSxpr(const struct sexpr *root,
                 virDomainDiskDefPtr disk;
                 if (VIR_ALLOC(disk) < 0)
                     goto error;
-                if (VIR_STRDUP(disk->src, tmp) < 0) {
+                if (virDomainDiskSetSource(disk, tmp) < 0) {
                     VIR_FREE(disk);
                     goto error;
                 }
-                disk->type = VIR_DOMAIN_DISK_TYPE_FILE;
+                virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE);
                 disk->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY;
                 if (VIR_STRDUP(disk->dst, fds[i]) < 0) {
                     virDomainDiskDefFree(disk);
                     goto error;
                 }
-                if (VIR_STRDUP(disk->driverName, "file") < 0) {
+                if (virDomainDiskSetSource(disk, "file") < 0) {
                     virDomainDiskDefFree(disk);
                     goto error;
                 }
@@ -1722,6 +1721,9 @@ xenFormatSxprDisk(virDomainDiskDefPtr def,
                   int xendConfigVersion,
                   int isAttach)
 {
+    const char *src = virDomainDiskGetSource(def);
+    const char *driver = virDomainDiskGetDriver(def);
+
     /* Xend (all versions) put the floppy device config
      * under the hvm (image (os)) block
      */
@@ -1729,7 +1731,7 @@ xenFormatSxprDisk(virDomainDiskDefPtr def,
         def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
         if (isAttach) {
             virReportError(VIR_ERR_INVALID_ARG,
-                           _("Cannot directly attach floppy %s"),
def->src);
+                           _("Cannot directly attach floppy %s"), src);
             return -1;
         }
         return 0;
@@ -1741,7 +1743,7 @@ xenFormatSxprDisk(virDomainDiskDefPtr def,
         xendConfigVersion == XEND_CONFIG_VERSION_3_0_2) {
         if (isAttach) {
             virReportError(VIR_ERR_INVALID_ARG,
-                           _("Cannot directly attach CDROM %s"), def->src);
+                           _("Cannot directly attach CDROM %s"), src);
             return -1;
         }
         return 0;
@@ -1753,9 +1755,9 @@ xenFormatSxprDisk(virDomainDiskDefPtr def,
     /* Normally disks are in a (device (vbd ...)) block
      * but blktap disks ended up in a differently named
      * (device (tap ....)) block.... */
-    if (def->driverName && STREQ(def->driverName, "tap")) {
+    if (STREQ_NULLABLE(driver, "tap")) {
         virBufferAddLit(buf, "(tap ");
-    } else if (def->driverName && STREQ(def->driverName, "tap2"))
{
+    } else if (STREQ_NULLABLE(driver, "tap2")) {
         virBufferAddLit(buf, "(tap2 ");
     } else {
         virBufferAddLit(buf, "(vbd ");
@@ -1778,36 +1780,39 @@ xenFormatSxprDisk(virDomainDiskDefPtr def,
         virBufferEscapeSexpr(buf, "(dev '%s')", def->dst);
     }

-    if (def->src) {
-        if (def->driverName) {
-            if (STREQ(def->driverName, "tap") ||
-                STREQ(def->driverName, "tap2")) {
+    if (src) {
+        if (driver) {
+            if (STREQ(driver, "tap") ||
+                STREQ(driver, "tap2")) {
                 const char *type;
+                int format = virDomainDiskGetFormat(def);

-                if (!def->format || def->format == VIR_STORAGE_FILE_RAW)
+                if (!format || format == VIR_STORAGE_FILE_RAW)
                     type = "aio";
                 else
-                    type = virStorageFileFormatTypeToString(def->format);
-                virBufferEscapeSexpr(buf, "(uname '%s:",
def->driverName);
+                    type = virStorageFileFormatTypeToString(format);
+                virBufferEscapeSexpr(buf, "(uname '%s:", driver);
                 virBufferEscapeSexpr(buf, "%s:", type);
-                virBufferEscapeSexpr(buf, "%s')", def->src);
+                virBufferEscapeSexpr(buf, "%s')", src);
             } else {
-                virBufferEscapeSexpr(buf, "(uname '%s:",
def->driverName);
-                virBufferEscapeSexpr(buf, "%s')", def->src);
+                virBufferEscapeSexpr(buf, "(uname '%s:", driver);
+                virBufferEscapeSexpr(buf, "%s')", src);
             }
         } else {
-            if (def->type == VIR_DOMAIN_DISK_TYPE_FILE) {
-                virBufferEscapeSexpr(buf, "(uname 'file:%s')",
def->src);
-            } else if (def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) {
-                if (def->src[0] == '/')
-                    virBufferEscapeSexpr(buf, "(uname 'phy:%s')",
def->src);
+            int type = virDomainDiskGetType(def);
+
+            if (type == VIR_DOMAIN_DISK_TYPE_FILE) {
+                virBufferEscapeSexpr(buf, "(uname 'file:%s')", src);
+            } else if (type == VIR_DOMAIN_DISK_TYPE_BLOCK) {
+                if (src[0] == '/')
+                    virBufferEscapeSexpr(buf, "(uname 'phy:%s')",
src);
                 else
                     virBufferEscapeSexpr(buf, "(uname 'phy:/dev/%s')",
-                                         def->src);
+                                         src);
             } else {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                _("unsupported disk type %s"),
-                               virDomainDiskTypeToString(def->type));
+                               virDomainDiskTypeToString(type));
                 return -1;
             }
         }
@@ -2313,23 +2318,23 @@ xenFormatSxpr(virConnectPtr conn,

             /* some disk devices are defined here */
             for (i = 0; i < def->ndisks; i++) {
+                const char *src = virDomainDiskGetSource(def->disks[i]);
+
                 switch (def->disks[i]->device) {
                 case VIR_DOMAIN_DISK_DEVICE_CDROM:
                     /* Only xend <= 3.0.2 wants cdrom config here */
                     if (xendConfigVersion != XEND_CONFIG_VERSION_3_0_2)
                         break;
-                    if (!STREQ(def->disks[i]->dst, "hdc") ||
-                        def->disks[i]->src == NULL)
+                    if (!STREQ(def->disks[i]->dst, "hdc") || !src)
                         break;

-                    virBufferEscapeSexpr(&buf, "(cdrom '%s')",
-                                         def->disks[i]->src);
+                    virBufferEscapeSexpr(&buf, "(cdrom '%s')",
src);
                     break;

                 case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
                     /* all xend versions define floppies here */
                     virBufferEscapeSexpr(&buf, "(%s ",
def->disks[i]->dst);
-                    virBufferEscapeSexpr(&buf, "'%s')",
def->disks[i]->src);
+                    virBufferEscapeSexpr(&buf, "'%s')", src);
                     break;

                 default:
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index a70c5e3..d74e427 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1,7 +1,7 @@
 /*
  * xen_xm.c: Xen XM parsing functions
  *
- * Copyright (C) 2006-2007, 2009-2010, 2012-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2014 Red Hat, Inc.
  * Copyright (C) 2011 Univention GmbH
  * Copyright (C) 2006 Daniel P. Berrange
  *
@@ -472,6 +472,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
             char *head;
             char *offset;
             char *tmp;
+            const char *src;

             if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
                 goto skipdisk;
@@ -493,17 +494,16 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
                 goto skipdisk;

             if (offset == head) {
-                disk->src = NULL; /* No source file given, eg CDROM with no media */
+                /* No source file given, eg CDROM with no media */
+                ignore_value(virDomainDiskSetSource(disk, NULL));
             } else {
-                if (VIR_ALLOC_N(disk->src, (offset - head) + 1) < 0)
+                if (VIR_STRNDUP(tmp, head, offset - head) < 0)
                     goto cleanup;
-                if (virStrncpy(disk->src, head, offset - head,
-                               (offset - head) + 1) == NULL) {
-                    virReportError(VIR_ERR_INTERNAL_ERROR,
-                                   _("Source file %s too big for
destination"),
-                                   head);
+                if (virDomainDiskSetSource(disk, tmp) < 0) {
+                    VIR_FREE(tmp);
                     goto cleanup;
                 }
+                VIR_FREE(tmp);
             }
             head = offset + 1;

@@ -524,65 +524,68 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
             }
             head = offset + 1;

-
             /* Extract source driver type */
-            if (disk->src) {
+            src = virDomainDiskGetSource(disk);
+            if (src) {
+                size_t len;
                 /* The main type  phy:, file:, tap: ... */
-                if ((tmp = strchr(disk->src, ':')) != NULL) {
-                    if (VIR_ALLOC_N(disk->driverName, (tmp - disk->src) + 1) <
0)
+                if ((tmp = strchr(src, ':')) != NULL) {
+                    len = tmp - src;
+                    if (VIR_STRNDUP(tmp, src, len) < 0)
                         goto cleanup;
-                    if (virStrncpy(disk->driverName, disk->src,
-                                   (tmp - disk->src),
-                                   (tmp - disk->src) + 1) == NULL) {
-                        virReportError(VIR_ERR_INTERNAL_ERROR,
-                                       _("Driver name %s too big for
destination"),
-                                       disk->src);
+                    if (virDomainDiskSetDriver(disk, tmp) < 0) {
+                        VIR_FREE(tmp);
                         goto cleanup;
                     }
+                    VIR_FREE(tmp);

                     /* Strip the prefix we found off the source file name */
-                    memmove(disk->src, disk->src+(tmp-disk->src)+1,
-                            strlen(disk->src)-(tmp-disk->src));
+                    if (virDomainDiskSetSource(disk, src + len + 1) < 0)
+                        goto cleanup;
+                    src = virDomainDiskGetSource(disk);
                 }

                 /* And the sub-type for tap:XXX: type */
-                if (disk->driverName &&
-                    STREQ(disk->driverName, "tap")) {
+                if (STREQ_NULLABLE(virDomainDiskGetDriver(disk), "tap")) {
                     char *driverType;

-                    if (!(tmp = strchr(disk->src, ':')))
+                    if (!(tmp = strchr(src, ':')))
                         goto skipdisk;
+                    len = tmp - src;

-                    if (VIR_STRNDUP(driverType, disk->src, tmp - disk->src) <
0)
+                    if (VIR_STRNDUP(driverType, src, len) < 0)
                         goto cleanup;
                     if (STREQ(driverType, "aio"))
-                        disk->format = VIR_STORAGE_FILE_RAW;
+                        virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
                     else
-                        disk->format =
-                            virStorageFileFormatTypeFromString(driverType);
+                        virDomainDiskSetFormat(disk,
+                                              
virStorageFileFormatTypeFromString(driverType));
                     VIR_FREE(driverType);
-                    if (disk->format <= 0) {
+                    if (virDomainDiskGetFormat(disk) <= 0) {
                         virReportError(VIR_ERR_INTERNAL_ERROR,
                                        _("Unknown driver type %s"),
-                                       disk->src);
+                                       src);
                         goto cleanup;
                     }

                     /* Strip the prefix we found off the source file name */
-                    memmove(disk->src, disk->src+(tmp-disk->src)+1,
-                            strlen(disk->src)-(tmp-disk->src));
+                    if (virDomainDiskSetSource(disk, src + len + 1) < 0)
+                        goto cleanup;
+                    src = virDomainDiskGetSource(disk);
                 }
             }

             /* No source, or driver name, so fix to phy: */
-            if (!disk->driverName &&
-                VIR_STRDUP(disk->driverName, "phy") < 0)
+            if (!virDomainDiskGetDriver(disk) &&
+                virDomainDiskSetDriver(disk, "phy") < 0)
                 goto cleanup;


             /* phy: type indicates a block device */
-            disk->type = STREQ(disk->driverName, "phy") ?
-                VIR_DOMAIN_DISK_TYPE_BLOCK : VIR_DOMAIN_DISK_TYPE_FILE;
+            virDomainDiskSetType(disk,
+                                 STREQ(virDomainDiskGetDriver(disk), "phy") ?
+                                 VIR_DOMAIN_DISK_TYPE_BLOCK :
+                                 VIR_DOMAIN_DISK_TYPE_FILE);

             /* Check for a :cdrom/:disk postfix */
             disk->device = VIR_DOMAIN_DISK_DEVICE_DISK;
@@ -624,11 +627,11 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
             if (VIR_ALLOC(disk) < 0)
                 goto cleanup;

-            disk->type = VIR_DOMAIN_DISK_TYPE_FILE;
+            virDomainDiskSetType(disk, VIR_DOMAIN_DISK_TYPE_FILE);
             disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
-            if (VIR_STRDUP(disk->driverName, "file") < 0)
+            if (virDomainDiskSetDriver(disk, "file") < 0)
                 goto cleanup;
-            if (VIR_STRDUP(disk->src, str) < 0)
+            if (virDomainDiskSetSource(disk, str) < 0)
                 goto cleanup;
             if (VIR_STRDUP(disk->dst, "hdc") < 0)
                 goto cleanup;
@@ -1176,27 +1179,31 @@ int xenXMConfigSetString(virConfPtr conf, const char *setting,
const char *str)
 }


-static int xenFormatXMDisk(virConfValuePtr list,
-                                       virDomainDiskDefPtr disk,
-                                       int hvm,
-                                       int xendConfigVersion)
+static int
+xenFormatXMDisk(virConfValuePtr list,
+                virDomainDiskDefPtr disk,
+                int hvm,
+                int xendConfigVersion)
 {
     virBuffer buf = VIR_BUFFER_INITIALIZER;
     virConfValuePtr val, tmp;
+    const char *src = virDomainDiskGetSource(disk);
+    int format = virDomainDiskGetFormat(disk);
+    const char *driver = virDomainDiskGetDriver(disk);

-    if (disk->src) {
-        if (disk->format) {
+    if (src) {
+        if (format) {
             const char *type;

-            if (disk->format == VIR_STORAGE_FILE_RAW)
+            if (format == VIR_STORAGE_FILE_RAW)
                 type = "aio";
             else
-                type = virStorageFileFormatTypeToString(disk->format);
-            virBufferAsprintf(&buf, "%s:", disk->driverName);
-            if (STREQ(disk->driverName, "tap"))
+                type = virStorageFileFormatTypeToString(format);
+            virBufferAsprintf(&buf, "%s:", driver);
+            if (STREQ(driver, "tap"))
                 virBufferAsprintf(&buf, "%s:", type);
         } else {
-            switch (disk->type) {
+            switch (virDomainDiskGetType(disk)) {
             case VIR_DOMAIN_DISK_TYPE_FILE:
                 virBufferAddLit(&buf, "file:");
                 break;
@@ -1206,11 +1213,11 @@ static int xenFormatXMDisk(virConfValuePtr list,
             default:
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("unsupported disk type %s"),
-                               virDomainDiskTypeToString(disk->type));
+                               virDomainDiskTypeToString(virDomainDiskGetType(disk)));
                 goto cleanup;
             }
         }
-        virBufferAdd(&buf, disk->src, -1);
+        virBufferAdd(&buf, src, -1);
     }
     virBufferAddLit(&buf, ",");
     if (hvm && xendConfigVersion == XEND_CONFIG_VERSION_3_0_2)
@@ -1602,9 +1609,9 @@ virConfPtr xenFormatXM(virConnectPtr conn,
                 if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM
&&
                     def->disks[i]->dst &&
                     STREQ(def->disks[i]->dst, "hdc") &&
-                    def->disks[i]->src) {
+                    virDomainDiskGetSource(def->disks[i])) {
                     if (xenXMConfigSetString(conf, "cdrom",
-                                             def->disks[i]->src) < 0)
+                                             virDomainDiskGetSource(def->disks[i]))
< 0)
                         goto cleanup;
                     break;
                 }
-- 
1.8.5.3


    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

[libvirt] [PATCH 17/18] conf: use disk source accessors in xenxs/