[PATCH] virconf.c: Refactor cleanup and remove VIR_FREE

[Yi Li]

Switch to using the 'g_auto*' helpers.

Signed-off-by: Yi Li 
---
 src/util/virconf.c | 47 +++---
 1 file changed, 15 insertions(+), 32 deletions(-)

diff --git a/src/util/virconf.c b/src/util/virconf.c
index 16107bce96..17fbea2397 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -573,7 +573,7 @@ static int
 virConfParseComment(virConfParserCtxtPtr ctxt)
 {
 const char *base;
-char *comm;
+g_autofree char *comm;
 
 if (CUR != '#')
 return -1;
@@ -581,10 +581,9 @@ virConfParseComment(virConfParserCtxtPtr ctxt)
 base = ctxt->cur;
 while ((ctxt->cur < ctxt->end) && (!IS_EOL(CUR))) NEXT;
 comm = g_strndup(base, ctxt->cur - base);
-if (virConfAddEntry(ctxt->conf, NULL, NULL, comm) == NULL) {
-VIR_FREE(comm);
+if (virConfAddEntry(ctxt->conf, NULL, NULL, comm) == NULL)
 return -1;
-}
+
 return 0;
 }
 
@@ -626,9 +625,9 @@ static int
 virConfParseStatement(virConfParserCtxtPtr ctxt)
 {
 const char *base;
-char *name;
+g_autofree char *name;
 virConfValuePtr value;
-char *comm = NULL;
+g_autofree char *comm = NULL;
 
 SKIP_BLANKS_AND_EOL;
 if (CUR == '#')
@@ -639,16 +638,13 @@ virConfParseStatement(virConfParserCtxtPtr ctxt)
 SKIP_BLANKS;
 if (CUR != '=') {
 virConfError(ctxt, VIR_ERR_CONF_SYNTAX, _("expecting an assignment"));
-VIR_FREE(name);
 return -1;
 }
 NEXT;
 SKIP_BLANKS;
 value = virConfParseValue(ctxt);
-if (value == NULL) {
-VIR_FREE(name);
+if (value == NULL)
 return -1;
-}
 SKIP_BLANKS;
 if (CUR == '#') {
 NEXT;
@@ -657,9 +653,7 @@ virConfParseStatement(virConfParserCtxtPtr ctxt)
 comm = g_strndup(base, ctxt->cur - base);
 }
 if (virConfAddEntry(ctxt->conf, name, value, comm) == NULL) {
-VIR_FREE(name);
 virConfFreeValue(value);
-VIR_FREE(comm);
 return -1;
 }
 return 0;
@@ -724,7 +718,7 @@ virConfParse(const char *filename, const char *content, int 
len,
 virConfPtr
 virConfReadFile(const char *filename, unsigned int flags)
 {
-char *content;
+g_autofree char *content;
 int len;
 virConfPtr conf;
 
@@ -740,7 +734,6 @@ virConfReadFile(const char *filename, unsigned int flags)
 
 conf = virConfParse(filename, content, len, flags);
 
-VIR_FREE(content);
 
 return conf;
 }
@@ -1412,7 +1405,7 @@ virConfWriteFile(const char *filename, virConfPtr conf)
 virConfEntryPtr cur;
 int ret;
 int fd;
-char *content;
+g_autofree char *content;
 unsigned int use;
 
 if (conf == NULL)
@@ -1433,7 +1426,6 @@ virConfWriteFile(const char *filename, virConfPtr conf)
 use = virBufferUse();
 content = virBufferContentAndReset();
 ret = safewrite(fd, content, use);
-VIR_FREE(content);
 VIR_FORCE_CLOSE(fd);
 if (ret != (int)use) {
 virConfError(NULL, VIR_ERR_WRITE_FAILED, _("failed to save content"));
@@ -1461,7 +1453,7 @@ virConfWriteMem(char *memory, int *len, virConfPtr conf)
 {
 g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
 virConfEntryPtr cur;
-char *content;
+g_autofree char *content;
 unsigned int use;
 
 if ((memory == NULL) || (len == NULL) || (*len <= 0) || (conf == NULL))
@@ -1478,11 +1470,9 @@ virConfWriteMem(char *memory, int *len, virConfPtr conf)
 
 if ((int)use >= *len) {
 *len = (int)use;
-VIR_FREE(content);
 return -1;
 }
 memcpy(memory, content, use);
-VIR_FREE(content);
 *len = use;
 return use;
 }
@@ -1505,26 +1495,19 @@ virConfLoadConfigPath(const char *name)
 int
 virConfLoadConfig(virConfPtr *conf, const char *name)
 {
-char *path = NULL;
-int ret = -1;
+g_autofree char *path = NULL;
 
 *conf = NULL;
 
 if (!(path = virConfLoadConfigPath(name)))
-goto cleanup;
+return -1;
 
-if (!virFileExists(path)) {
-ret = 0;
-goto cleanup;
-}
+if (!virFileExists(path))
+return 0;
 
 VIR_DEBUG("Loading config file '%s'", path);
 if (!(*conf = virConfReadFile(path, 0)))
-goto cleanup;
-
-ret = 0;
+return -1;
 
- cleanup:
-VIR_FREE(path);
-return ret;
+return 0;
 }
-- 
2.25.3

[PATCH] virConfLoadConfig: Refactor cleanup

[Yi Li]

Switch to using the 'g_auto*' helpers.

Signed-off-by: Yi Li 
---
 src/util/virconf.c | 19 ++-
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/src/util/virconf.c b/src/util/virconf.c
index 16107bce96..11046a1d45 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -1505,26 +1505,19 @@ virConfLoadConfigPath(const char *name)
 int
 virConfLoadConfig(virConfPtr *conf, const char *name)
 {
-char *path = NULL;
-int ret = -1;
+g_autofree char *path = NULL;
 
 *conf = NULL;
 
 if (!(path = virConfLoadConfigPath(name)))
-goto cleanup;
+return -1;
 
-if (!virFileExists(path)) {
-ret = 0;
-goto cleanup;
-}
+if (!virFileExists(path))
+return 0;
 
 VIR_DEBUG("Loading config file '%s'", path);
 if (!(*conf = virConfReadFile(path, 0)))
-goto cleanup;
-
-ret = 0;
+return -1;
 
- cleanup:
-VIR_FREE(path);
-return ret;
+return 0;
 }
-- 
2.25.3

[PATCH] virQEMUCapsInitQMPArch: Refactor cleanup

[Yi Li]

Switch to using the 'g_auto*' helpers.

Signed-off-by: Yi Li 
---
 src/qemu/qemu_capabilities.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 790375ac38..447cf77875 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5044,23 +5044,18 @@ static int
 virQEMUCapsInitQMPArch(virQEMUCapsPtr qemuCaps,
 qemuMonitorPtr mon)
 {
-char *archstr = NULL;
-int ret = -1;
+g_autofree char *archstr = NULL;
 
 if (!(archstr = qemuMonitorGetTargetArch(mon)))
-goto cleanup;
+return -1;
 
 if ((qemuCaps->arch = virQEMUCapsArchFromString(archstr)) == 
VIR_ARCH_NONE) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unknown QEMU arch %s"), archstr);
-goto cleanup;
+return -1;
 }
 
-ret = 0;
-
- cleanup:
-VIR_FREE(archstr);
-return ret;
+return 0;
 }
 
 
-- 
2.25.3

[PATCH] qemuBlockDiskDetectNodes: just return when alias is null

[Yi Li]

Just return when alias is null and Remove the 'ret' variable.

Signed-off-by: Yi Li 
---
 src/qemu/qemu_block.c | 21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 3d88e701b2..0af3e56c9a 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -280,25 +280,22 @@ qemuBlockDiskDetectNodes(virDomainDiskDefPtr disk,
 qemuBlockNodeNameBackingChainDataPtr entry = NULL;
 virStorageSourcePtr src = disk->src;
 g_autofree char *alias = NULL;
-int ret = -1;
 
 /* don't attempt the detection if the top level already has node names */
 if (src->nodeformat || src->nodestorage)
 return 0;
 
 if (!(alias = qemuAliasDiskDriveFromDisk(disk)))
-goto cleanup;
+return -1;
 
-if (!(entry = virHashLookup(disktable, alias))) {
-ret = 0;
-goto cleanup;
-}
+if (!(entry = virHashLookup(disktable, alias)))
+return 0;
 
 while (virStorageSourceIsBacking(src) && entry) {
 if (src->nodeformat || src->nodestorage) {
 if (STRNEQ_NULLABLE(src->nodeformat, entry->nodeformat) ||
 STRNEQ_NULLABLE(src->nodestorage, entry->nodestorage))
-goto cleanup;
+goto error;
 
 break;
 } else {
@@ -310,13 +307,11 @@ qemuBlockDiskDetectNodes(virDomainDiskDefPtr disk,
 src = src->backingStore;
 }
 
-ret = 0;
-
- cleanup:
-if (ret < 0)
-qemuBlockDiskClearDetectedNodes(disk);
+return 0;
 
-return ret;
+ error:
+qemuBlockDiskClearDetectedNodes(disk);
+return -1;
 }
 
 
-- 
2.25.3

[PATCH] Rework qemuMigrationDstPrecreateDisk()

[Yi Li]

'conn' vairable which are used only inside the func. Let's declare
inside the func body to make that obvious.
Use g_autofree to allow removal of 'cleanup:' and the 'ret' variable.

Signed-off-by: Yi Li 
---
 src/qemu/qemu_migration.c | 68 +++
 1 file changed, 26 insertions(+), 42 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index f44d31c971..6bb0677f86 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -169,15 +169,15 @@ qemuMigrationSrcRestoreDomainState(virQEMUDriverPtr 
driver, virDomainObjPtr vm)
 
 
 static int
-qemuMigrationDstPrecreateDisk(virConnectPtr *conn,
-  virDomainDiskDefPtr disk,
+qemuMigrationDstPrecreateDisk(virDomainDiskDefPtr disk,
   unsigned long long capacity)
 {
-int ret = -1;
-virStoragePoolPtr pool = NULL;
-virStorageVolPtr vol = NULL;
-char *volName = NULL, *basePath = NULL;
-char *volStr = NULL;
+g_autoptr(virConnect) conn = NULL;
+g_autoptr(virStoragePool) pool = NULL;
+g_autoptr(virStorageVol) vol = NULL;
+char *volName = NULL;
+g_autofree char *basePath = NULL;
+g_autofree char *volStr = NULL;
 g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
 const char *format = NULL;
 unsigned int flags = 0;
@@ -198,32 +198,28 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn,
 virReportError(VIR_ERR_INVALID_ARG,
_("malformed disk path: %s"),
disk->src->path);
-goto cleanup;
+return -1;
 }
 
 *volName = '\0';
 volName++;
 
-if (!*conn) {
-if (!(*conn = virGetConnectStorage()))
-goto cleanup;
-}
+if (!(conn = virGetConnectStorage()))
+return -1;
 
-if (!(pool = virStoragePoolLookupByTargetPath(*conn, basePath)))
-goto cleanup;
+if (!(pool = virStoragePoolLookupByTargetPath(conn, basePath)))
+return -1;
 format = virStorageFileFormatTypeToString(disk->src->format);
 if (disk->src->format == VIR_STORAGE_FILE_QCOW2)
 flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
 break;
 
 case VIR_STORAGE_TYPE_VOLUME:
-if (!*conn) {
-if (!(*conn = virGetConnectStorage()))
-goto cleanup;
-}
+if (!(conn = virGetConnectStorage()))
+return -1;
 
-if (!(pool = virStoragePoolLookupByName(*conn, 
disk->src->srcpool->pool)))
-goto cleanup;
+if (!(pool = virStoragePoolLookupByName(conn, 
disk->src->srcpool->pool)))
+return -1;
 format = virStorageFileFormatTypeToString(disk->src->format);
 volName = disk->src->srcpool->volume;
 if (disk->src->format == VIR_STORAGE_FILE_QCOW2)
@@ -245,13 +241,13 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn,
_("cannot precreate storage for disk type '%s'"),
virStorageTypeToString(disk->src->type));
 goto cleanup;
+return -1;
 }
 
 if ((vol = virStorageVolLookupByName(pool, volName))) {
 VIR_DEBUG("Skipping creation of already existing volume of name '%s'",
   volName);
-ret = 0;
-goto cleanup;
+return 0;
 }
 
 virBufferAddLit(, "\n");
@@ -269,19 +265,13 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn,
 if (!(volStr = virBufferContentAndReset())) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("unable to create volume XML"));
-goto cleanup;
+return -1;
 }
 
 if (!(vol = virStorageVolCreateXML(pool, volStr, flags)))
-goto cleanup;
+return -1;
 
-ret = 0;
- cleanup:
-VIR_FREE(basePath);
-VIR_FREE(volStr);
-virObjectUnref(vol);
-virObjectUnref(pool);
-return ret;
+return 0;
 }
 
 static bool
@@ -313,9 +303,7 @@ qemuMigrationDstPrecreateStorage(virDomainObjPtr vm,
  const char **migrate_disks,
  bool incremental)
 {
-int ret = -1;
 size_t i = 0;
-virConnectPtr conn = NULL;
 
 if (!nbd || !nbd->ndisks)
 return 0;
@@ -332,7 +320,7 @@ qemuMigrationDstPrecreateStorage(virDomainObjPtr vm,
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("unable to find disk by target: %s"),
nbd->disks[i].target);
-goto cleanup;
+return -1;
 }
 
 if (disk->src->type == VIR_STORAGE_TYPE_NVME) {
@@ -352,20 +340,16 @@ qemuMigrationDstPrecreateStorage(virDomainObjPtr vm,
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%

[PATCH] qemuBlockDiskDetectNodes: just return when alias is null

[Yi Li]

Just Return when alias is null and Remove the 'ret' variable.

Signed-off-by: Yi Li 
---
 src/qemu/qemu_block.c | 21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 0b8ca2a3f5..32b6500a3d 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -280,25 +280,22 @@ qemuBlockDiskDetectNodes(virDomainDiskDefPtr disk,
 qemuBlockNodeNameBackingChainDataPtr entry = NULL;
 virStorageSourcePtr src = disk->src;
 g_autofree char *alias = NULL;
-int ret = -1;
 
 /* don't attempt the detection if the top level already has node names */
 if (src->nodeformat || src->nodestorage)
 return 0;
 
 if (!(alias = qemuAliasDiskDriveFromDisk(disk)))
-goto cleanup;
+return 0;
 
-if (!(entry = virHashLookup(disktable, alias))) {
-ret = 0;
-goto cleanup;
-}
+if (!(entry = virHashLookup(disktable, alias)))
+return 0;
 
 while (virStorageSourceIsBacking(src) && entry) {
 if (src->nodeformat || src->nodestorage) {
 if (STRNEQ_NULLABLE(src->nodeformat, entry->nodeformat) ||
 STRNEQ_NULLABLE(src->nodestorage, entry->nodestorage))
-goto cleanup;
+goto error;
 
 break;
 } else {
@@ -310,13 +307,11 @@ qemuBlockDiskDetectNodes(virDomainDiskDefPtr disk,
 src = src->backingStore;
 }
 
-ret = 0;
-
- cleanup:
-if (ret < 0)
-qemuBlockDiskClearDetectedNodes(disk);
+return 0;
 
-return ret;
+ error:
+qemuBlockDiskClearDetectedNodes(disk);
+return -1;
 }
 
 
-- 
2.25.3

[PATCH] qemuDomainAttachRedirdevDevice: Remove need_release variable

[Yi Li]

Get rid of the 'need_release' variable.

Signed-off-by: Yi Li 
---
 src/qemu/qemu_hotplug.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 882e5d2384..e07dba3c5e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1921,18 +1921,16 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr 
driver,
 bool chardevAdded = false;
 g_autofree char *tlsAlias = NULL;
 const char *secAlias = NULL;
-bool need_release = false;
 virErrorPtr orig_err;
 
 if (qemuAssignDeviceRedirdevAlias(def, redirdev, -1) < 0)
-goto cleanup;
+return -1;
 
 if (!(charAlias = qemuAliasChardevFromDevAlias(redirdev->info.alias)))
-goto cleanup;
+return -1;
 
 if ((virDomainUSBAddressEnsure(priv->usbaddrs, >info)) < 0)
-goto cleanup;
-need_release = true;
+return -1;
 
 if (!(devstr = qemuBuildRedirdevDevStr(def, redirdev, priv->qemuCaps)))
 goto cleanup;
@@ -1964,7 +1962,7 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr 
driver,
  audit:
 virDomainAuditRedirdev(vm, redirdev, "attach", ret == 0);
  cleanup:
-if (ret < 0 && need_release)
+if (ret < 0)
 qemuDomainReleaseDeviceAddress(vm, >info);
 return ret;
 
-- 
2.25.3

[PATCH] storage: volStorageBackendRBDRefreshVolInfo: refactor

[Yi Li]

use the ret variable for return value

Signed-off-by: Yi Li 
---
 src/storage/storage_backend_rbd.c | 22 ++
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index 1630d6eede..22f5c78591 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -507,36 +507,30 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr 
vol,
virStoragePoolObjPtr pool,
virStorageBackendRBDStatePtr ptr)
 {
-int rc, ret = -1;
+int ret = -1;
 virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
 rbd_image_t image = NULL;
 rbd_image_info_t info;
 uint64_t features;
 uint64_t flags;
 
-if ((rc = rbd_open_read_only(ptr->ioctx, vol->name, , NULL)) < 0) {
-ret = rc;
+if ((ret = rbd_open_read_only(ptr->ioctx, vol->name, , NULL)) < 0) {
 virReportSystemError(errno, _("failed to open the RBD image '%s'"),
  vol->name);
 goto cleanup;
 }
 
-if ((rc = rbd_stat(image, , sizeof(info))) < 0) {
-ret = rc;
+if ((ret = rbd_stat(image, , sizeof(info))) < 0) {
 virReportSystemError(errno, _("failed to stat the RBD image '%s'"),
  vol->name);
 goto cleanup;
 }
 
-if ((rc = volStorageBackendRBDGetFeatures(image, vol->name, )) < 
0) {
-ret = rc;
+if ((ret = volStorageBackendRBDGetFeatures(image, vol->name, )) < 
0)
 goto cleanup;
-}
 
-if ((rc = volStorageBackendRBDGetFlags(image, vol->name, )) < 0) {
-ret = rc;
+if ((ret = volStorageBackendRBDGetFlags(image, vol->name, )) < 0)
 goto cleanup;
-}
 
 vol->target.capacity = info.size;
 vol->type = VIR_STORAGE_VOL_NETWORK;
@@ -549,10 +543,8 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol,
   "Querying for actual allocation",
   def->source.name, vol->name);
 
-if ((rc = virStorageBackendRBDSetAllocation(vol, image, )) < 0) {
-ret = rc;
+if ((ret = virStorageBackendRBDSetAllocation(vol, image, )) < 0)
 goto cleanup;
-}
 } else {
 vol->target.allocation = info.obj_size * info.num_objs;
 }
@@ -568,8 +560,6 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol,
 VIR_FREE(vol->key);
 vol->key = g_strdup_printf("%s/%s", def->source.name, vol->name);
 
-ret = 0;
-
  cleanup:
 if (image)
 rbd_close(image);
-- 
2.25.3

[PATCH] storageBackendCreatePloop: Refactor cleanup

[Yi Li]

get rid of the 'cleanup:' label and created variable.

Signed-off-by: Yi Li 
---
 src/storage/storage_util.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 2478cb0a4a..2e2c7dc68a 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -563,7 +563,6 @@ storageBackendCreatePloop(virStoragePoolObjPtr pool 
G_GNUC_UNUSED,
   unsigned int flags)
 {
 int ret = -1;
-bool created = false;
 g_autoptr(virCommand) cmd = NULL;
 g_autofree char *create_tool = NULL;
 
@@ -607,7 +606,7 @@ storageBackendCreatePloop(virStoragePoolObjPtr pool 
G_GNUC_UNUSED,
   0)) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("error creating directory for ploop volume"));
-goto cleanup;
+return -1;
 }
 cmd = virCommandNewArgList(create_tool, "init", "-s", NULL);
 virCommandAddArgFormat(cmd, "%lluM", VIR_DIV_UP(vol->target.capacity,
@@ -620,10 +619,8 @@ storageBackendCreatePloop(virStoragePoolObjPtr pool 
G_GNUC_UNUSED,
 cmd = virCommandNewArgList("cp", "-r", inputvol->target.path,
vol->target.path, NULL);
 }
-created = true;
 ret = virCommandRun(cmd, NULL);
- cleanup:
-if (ret < 0 && created)
+if (ret < 0)
 virFileDeleteTree(vol->target.path);
 return ret;
 }
-- 
2.25.3

[PATCH v2 2/3] virStorageBackendCopyToFD: remove unused return variable

[Yi Li]

remove unused return variable,
The errno will throw by virReportSystemError

Signed-off-by: Yi Li 
---
 src/storage/storage_util.c | 22 +++---
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index c13abf03af..c6d0f7a97c 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -128,7 +128,6 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
   bool reflink_copy)
 {
 int amtread = -1;
-int ret = 0;
 size_t rbytes = READ_BLOCK_SIZE_DEFAULT;
 int wbytes = 0;
 int interval;
@@ -138,11 +137,10 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
 VIR_AUTOCLOSE inputfd = -1;
 
 if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("could not open input path '%s'"),
  inputvol->target.path);
-return ret;
+return -1;
 }
 
 #ifdef __linux__
@@ -160,11 +158,10 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
 
 if (reflink_copy) {
 if (reflinkCloneFile(fd, inputfd) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("failed to clone files from '%s'"),
  inputvol->target.path);
-return ret;
+return -1;
 } else {
 VIR_DEBUG("btrfs clone finished.");
 return 0;
@@ -178,11 +175,10 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
 rbytes = *total;
 
 if ((amtread = saferead(inputfd, buf, rbytes)) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("failed reading from file '%s'"),
  inputvol->target.path);
-return ret;
+return -1;
 }
 *total -= amtread;
 
@@ -195,36 +191,32 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
 
 if (want_sparse && memcmp(buf+offset, zerobuf, interval) == 0) {
 if (lseek(fd, interval, SEEK_CUR) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("cannot extend file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 }
 } else if (safewrite(fd, buf+offset, interval) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("failed writing to file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 
 }
 } while ((amtleft -= interval) > 0);
 }
 
 if (virFileDataSync(fd) < 0) {
-ret = -errno;
 virReportSystemError(errno, _("cannot sync data to file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 }
 
 if (VIR_CLOSE(inputfd) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("cannot close file '%s'"),
  inputvol->target.path);
-return ret;
+return -1;
 }
 
 return 0;
-- 
2.25.3

[PATCH v2 3/3] storageBackendCreateRaw: remove unused created

[Yi Li]

refactor and remove unused created variable.

Signed-off-by: Yi Li 
---
 src/storage/storage_util.c | 23 ++-
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index c6d0f7a97c..cc8189c3e0 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -384,11 +384,9 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
 unsigned int flags)
 {
 virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
-int ret = -1;
 int operation_flags;
 bool reflink_copy = false;
 mode_t open_mode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE;
-bool created = false;
 VIR_AUTOCLOSE fd = -1;
 
 virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA |
@@ -399,13 +397,13 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("metadata preallocation is not supported for raw "
  "volumes"));
-goto cleanup;
+return -1;
 }
 
 if (virStorageSourceHasBacking(>target)) {
 virReportError(VIR_ERR_NO_SUPPORT, "%s",
_("backing storage not supported for raw volumes"));
-goto cleanup;
+return -1;
 }
 
 if (flags & VIR_STORAGE_VOL_CREATE_REFLINK)
@@ -415,7 +413,7 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
 if (vol->target.encryption) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("storage pool does not support encrypted volumes"));
-goto cleanup;
+return -1;
 }
 
 operation_flags = VIR_FILE_OPEN_FORCE_MODE | VIR_FILE_OPEN_FORCE_OWNER;
@@ -434,26 +432,25 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
 virReportSystemError(-fd,
  _("Failed to create file '%s'"),
  vol->target.path);
-goto cleanup;
+return -1;
 }
-created = true;
 
 /* NB, COW flag can only be toggled when the file is zero-size,
  * so must go before the createRawFile call allocates payload */
 if (vol->target.nocow &&
 virFileSetCOW(vol->target.path, VIR_TRISTATE_BOOL_NO) < 0)
-goto cleanup;
+goto error;
 
-if ((ret = createRawFile(fd, vol, inputvol, reflink_copy)) < 0)
+if (createRawFile(fd, vol, inputvol, reflink_copy) < 0)
 /* createRawFile already reported the exact error. */
-ret = -1;
+goto error;
+return 0;
 
- cleanup:
-if (ret < 0 && created)
+ error:
 ignore_value(virFileRemove(vol->target.path,
vol->target.perms->uid,
vol->target.perms->gid));
-return ret;
+return -1;
 }
 
 
-- 
2.25.3

[PATCH v2 0/3] Storage: remove unused variable

[Yi Li]

refactor storageBackendCreateRaw and remove some unused variable.

Yi Li (3):
  createRawFile: remove unused return variable
  virStorageBackendCopyToFD: remove unused return variable
  storageBackendCreateRaw: remove unused created

 src/storage/storage_util.c | 66 +++---
 1 file changed, 25 insertions(+), 41 deletions(-)

-- 
2.25.3

[PATCH v2 1/3] createRawFile: remove unused return variable

[Yi Li]

remove unused return variable,
The errno will throw by virReportSystemError

Signed-off-by: Yi Li 
---
 src/storage/storage_util.c | 21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index cf1f33f177..c13abf03af 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -315,7 +315,6 @@ createRawFile(int fd, virStorageVolDefPtr vol,
   bool reflink_copy)
 {
 bool need_alloc = true;
-int ret = 0;
 unsigned long long pos = 0;
 
 /* If the new allocation is lower than the capacity of the original file,
@@ -327,11 +326,10 @@ createRawFile(int fd, virStorageVolDefPtr vol,
 /* Seek to the final size, so the capacity is available upfront
  * for progress reporting */
 if (ftruncate(fd, vol->target.capacity) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("cannot extend file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 }
 
 /* Avoid issues with older kernel's  namespace pollution. */
@@ -347,11 +345,10 @@ createRawFile(int fd, virStorageVolDefPtr vol,
 if (fallocate(fd, 0, 0, vol->target.allocation) == 0) {
 need_alloc = false;
 } else if (errno != ENOSYS && errno != EOPNOTSUPP) {
-ret = -errno;
 virReportSystemError(errno,
  _("cannot allocate %llu bytes in file '%s'"),
  vol->target.allocation, vol->target.path);
-return ret;
+return -1;
 }
 }
 #endif
@@ -361,9 +358,9 @@ createRawFile(int fd, virStorageVolDefPtr vol,
 /* allow zero blocks to be skipped if we've requested sparse
  * allocation (allocation < capacity) or we have already
  * been able to allocate the required space. */
-if ((ret = virStorageBackendCopyToFD(vol, inputvol, fd, ,
- !need_alloc, reflink_copy)) < 0)
-return ret;
+if (virStorageBackendCopyToFD(vol, inputvol, fd, ,
+  !need_alloc, reflink_copy) < 0)
+return -1;
 
 /* If the new allocation is greater than the original capacity,
  * but fallocate failed, fill the rest with zeroes.
@@ -373,21 +370,19 @@ createRawFile(int fd, virStorageVolDefPtr vol,
 
 if (need_alloc && (vol->target.allocation - pos > 0)) {
 if (safezero(fd, pos, vol->target.allocation - pos) < 0) {
-ret = -errno;
 virReportSystemError(errno, _("cannot fill file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 }
 }
 
 if (g_fsync(fd) < 0) {
-ret = -errno;
 virReportSystemError(errno, _("cannot sync data to file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 }
 
-return ret;
+return 0;
 }
 
 static int
-- 
2.25.3

[PATCH 3/3] storageBackendCreateRaw: remove unused created variable

[Yi Li]

refactor and remove unused created variable

Signed-off-by: Yi Li 
---
 src/storage/storage_util.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index c6d0f7a97c..c02ece8253 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -384,11 +384,10 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
 unsigned int flags)
 {
 virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
-int ret = -1;
+int ret = 0;
 int operation_flags;
 bool reflink_copy = false;
 mode_t open_mode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE;
-bool created = false;
 VIR_AUTOCLOSE fd = -1;
 
 virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA |
@@ -399,13 +398,13 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("metadata preallocation is not supported for raw "
  "volumes"));
-goto cleanup;
+return -1;
 }
 
 if (virStorageSourceHasBacking(>target)) {
 virReportError(VIR_ERR_NO_SUPPORT, "%s",
_("backing storage not supported for raw volumes"));
-goto cleanup;
+return -1;
 }
 
 if (flags & VIR_STORAGE_VOL_CREATE_REFLINK)
@@ -415,7 +414,7 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
 if (vol->target.encryption) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("storage pool does not support encrypted volumes"));
-goto cleanup;
+return -1;
 }
 
 operation_flags = VIR_FILE_OPEN_FORCE_MODE | VIR_FILE_OPEN_FORCE_OWNER;
@@ -434,22 +433,23 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
 virReportSystemError(-fd,
  _("Failed to create file '%s'"),
  vol->target.path);
-goto cleanup;
+return -1;
 }
-created = true;
 
 /* NB, COW flag can only be toggled when the file is zero-size,
  * so must go before the createRawFile call allocates payload */
 if (vol->target.nocow &&
-virFileSetCOW(vol->target.path, VIR_TRISTATE_BOOL_NO) < 0)
+virFileSetCOW(vol->target.path, VIR_TRISTATE_BOOL_NO) < 0) {
+ret = -1;
 goto cleanup;
+}
 
-if ((ret = createRawFile(fd, vol, inputvol, reflink_copy)) < 0)
+if (createRawFile(fd, vol, inputvol, reflink_copy) < 0)
 /* createRawFile already reported the exact error. */
 ret = -1;
 
  cleanup:
-if (ret < 0 && created)
+if (ret < 0)
 ignore_value(virFileRemove(vol->target.path,
vol->target.perms->uid,
vol->target.perms->gid));
-- 
2.25.3

[PATCH 2/3] createRawFile: remove unused return variable

[Yi Li]

remove unused return variable,
The errno will throw by virReportSystemError

Signed-off-by: Yi Li 
---
 src/storage/storage_util.c | 21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 6fc8597733..c6d0f7a97c 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -307,7 +307,6 @@ createRawFile(int fd, virStorageVolDefPtr vol,
   bool reflink_copy)
 {
 bool need_alloc = true;
-int ret = 0;
 unsigned long long pos = 0;
 
 /* If the new allocation is lower than the capacity of the original file,
@@ -319,11 +318,10 @@ createRawFile(int fd, virStorageVolDefPtr vol,
 /* Seek to the final size, so the capacity is available upfront
  * for progress reporting */
 if (ftruncate(fd, vol->target.capacity) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("cannot extend file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 }
 
 /* Avoid issues with older kernel's  namespace pollution. */
@@ -339,11 +337,10 @@ createRawFile(int fd, virStorageVolDefPtr vol,
 if (fallocate(fd, 0, 0, vol->target.allocation) == 0) {
 need_alloc = false;
 } else if (errno != ENOSYS && errno != EOPNOTSUPP) {
-ret = -errno;
 virReportSystemError(errno,
  _("cannot allocate %llu bytes in file '%s'"),
  vol->target.allocation, vol->target.path);
-return ret;
+return -1;
 }
 }
 #endif
@@ -353,9 +350,9 @@ createRawFile(int fd, virStorageVolDefPtr vol,
 /* allow zero blocks to be skipped if we've requested sparse
  * allocation (allocation < capacity) or we have already
  * been able to allocate the required space. */
-if ((ret = virStorageBackendCopyToFD(vol, inputvol, fd, ,
- !need_alloc, reflink_copy)) < 0)
-return ret;
+if (virStorageBackendCopyToFD(vol, inputvol, fd, ,
+  !need_alloc, reflink_copy) < 0)
+return -1;
 
 /* If the new allocation is greater than the original capacity,
  * but fallocate failed, fill the rest with zeroes.
@@ -365,21 +362,19 @@ createRawFile(int fd, virStorageVolDefPtr vol,
 
 if (need_alloc && (vol->target.allocation - pos > 0)) {
 if (safezero(fd, pos, vol->target.allocation - pos) < 0) {
-ret = -errno;
 virReportSystemError(errno, _("cannot fill file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 }
 }
 
 if (g_fsync(fd) < 0) {
-ret = -errno;
 virReportSystemError(errno, _("cannot sync data to file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 }
 
-return ret;
+return 0;
 }
 
 static int
-- 
2.25.3

[PATCH 1/3] virStorageBackendCopyToFD: remove unused return variable

[Yi Li]

remove unused return variable,
The errno will throw by virReportSystemError

Signed-off-by: Yi Li 
---
 src/storage/storage_util.c | 22 +++---
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index cf1f33f177..6fc8597733 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -128,7 +128,6 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
   bool reflink_copy)
 {
 int amtread = -1;
-int ret = 0;
 size_t rbytes = READ_BLOCK_SIZE_DEFAULT;
 int wbytes = 0;
 int interval;
@@ -138,11 +137,10 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
 VIR_AUTOCLOSE inputfd = -1;
 
 if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("could not open input path '%s'"),
  inputvol->target.path);
-return ret;
+return -1;
 }
 
 #ifdef __linux__
@@ -160,11 +158,10 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
 
 if (reflink_copy) {
 if (reflinkCloneFile(fd, inputfd) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("failed to clone files from '%s'"),
  inputvol->target.path);
-return ret;
+return -1;
 } else {
 VIR_DEBUG("btrfs clone finished.");
 return 0;
@@ -178,11 +175,10 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
 rbytes = *total;
 
 if ((amtread = saferead(inputfd, buf, rbytes)) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("failed reading from file '%s'"),
  inputvol->target.path);
-return ret;
+return -1;
 }
 *total -= amtread;
 
@@ -195,36 +191,32 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
 
 if (want_sparse && memcmp(buf+offset, zerobuf, interval) == 0) {
 if (lseek(fd, interval, SEEK_CUR) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("cannot extend file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 }
 } else if (safewrite(fd, buf+offset, interval) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("failed writing to file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 
 }
 } while ((amtleft -= interval) > 0);
 }
 
 if (virFileDataSync(fd) < 0) {
-ret = -errno;
 virReportSystemError(errno, _("cannot sync data to file '%s'"),
  vol->target.path);
-return ret;
+return -1;
 }
 
 if (VIR_CLOSE(inputfd) < 0) {
-ret = -errno;
 virReportSystemError(errno,
  _("cannot close file '%s'"),
  inputvol->target.path);
-return ret;
+return -1;
 }
 
 return 0;
-- 
2.25.3

Re: [PATCH] util: xml: remove unused function virXMLChildElementCount

[Yi Li]

On 11/6/20, Ján Tomko  wrote:
> On a Friday in 2020, Yi Li wrote:
>>---
>> src/libvirt_private.syms |  1 -
>> src/util/virxml.c| 21 -
>> src/util/virxml.h|  1 -
>> 3 files changed, 23 deletions(-)
>>
>
> Looks good, but it's missing a sign-off.
>
> Can you, please, include a Signed-off-by line to indicate you complied
> with the Developer Certificate of Origin?
>

Signed-off-by: Yi Li 

> https://libvirt.org/hacking.html#developer-certificate-of-origin
>
> Replying to this e-mail with the line is enough, no need to resend the
> patch.
>

Thanks

> Jano
>

[PATCH] util: xml: remove unused function virXMLChildElementCount

[Yi Li]

---
 src/libvirt_private.syms |  1 -
 src/util/virxml.c| 21 -
 src/util/virxml.h|  1 -
 3 files changed, 23 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9029ea4fa2..36ab84162c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3504,7 +3504,6 @@ virVsockSetGuestCid;
 # util/virxml.h
 virParseScaledValue;
 virXMLCheckIllegalChars;
-virXMLChildElementCount;
 virXMLExtractNamespaceXML;
 virXMLFormatElement;
 virXMLNodeContentString;
diff --git a/src/util/virxml.c b/src/util/virxml.c
index 8a94de8fe4..a3b819d85c 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -934,27 +934,6 @@ virXMLSaveFile(const char *path,
 return virFileRewrite(path, S_IRUSR | S_IWUSR, virXMLRewriteFile, );
 }
 
-/* Returns the number of children of node, or -1 on error.  */
-long
-virXMLChildElementCount(xmlNodePtr node)
-{
-long ret = 0;
-xmlNodePtr cur = NULL;
-
-/* xmlChildElementCount returns 0 on error, which isn't helpful;
- * besides, it is not available in libxml2 2.6.  */
-if (!node || node->type != XML_ELEMENT_NODE)
-return -1;
-cur = node->children;
-while (cur) {
-if (cur->type == XML_ELEMENT_NODE)
-ret++;
-cur = cur->next;
-}
-return ret;
-}
-
-
 /**
  * virXMLNodeToString: convert an XML node ptr to an XML string
  *
diff --git a/src/util/virxml.h b/src/util/virxml.h
index b73591c3df..f73b8975ba 100644
--- a/src/util/virxml.h
+++ b/src/util/virxml.h
@@ -77,7 +77,6 @@ char * virXMLPropStringLimit(xmlNodePtr node,
  const char *name,
  size_t maxlen);
 char *   virXMLNodeContentString(xmlNodePtr node);
-long virXMLChildElementCount(xmlNodePtr node);
 
 /* Internal function; prefer the macros below.  */
 xmlDocPtr  virXMLParseHelper(int domcode,
-- 
2.25.3

[PATCH] virsh-domain: Remove unused virshNodeIsSuperset

[Yi Li]

The function is marked as unused. Remove it from the tree
until a new use case can be found.

Signed-off-by: Yi Li 
---
 tools/virsh-domain.c | 115 ---
 1 file changed, 115 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index ef347585e8..1ae936c6b2 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11933,121 +11933,6 @@ cmdDomHostname(vshControl *ctl, const vshCmd *cmd)
 return true;
 }
 
-/**
- * Check if n1 is superset of n2, meaning n1 contains all elements and
- * attributes as n2 at least. Including children.
- * @n1 first node
- * @n2 second node
- * returns true in case n1 covers n2, false otherwise.
- */
-G_GNUC_UNUSED
-static bool
-virshNodeIsSuperset(xmlNodePtr n1, xmlNodePtr n2)
-{
-xmlNodePtr child1, child2;
-xmlAttrPtr attr;
-char *prop1, *prop2;
-bool found;
-bool visited;
-bool ret = false;
-long n1_child_size, n2_child_size, n1_iter;
-virBitmapPtr bitmap;
-
-if (!n1 && !n2)
-return true;
-
-if (!n1 || !n2)
-return false;
-
-if (!xmlStrEqual(n1->name, n2->name))
-return false;
-
-/* Iterate over n2 attributes and check if n1 contains them */
-attr = n2->properties;
-while (attr) {
-if (attr->type == XML_ATTRIBUTE_NODE) {
-prop1 = virXMLPropString(n1, (const char *) attr->name);
-prop2 = virXMLPropString(n2, (const char *) attr->name);
-if (STRNEQ_NULLABLE(prop1, prop2)) {
-xmlFree(prop1);
-xmlFree(prop2);
-return false;
-}
-xmlFree(prop1);
-xmlFree(prop2);
-}
-attr = attr->next;
-}
-
-n1_child_size = virXMLChildElementCount(n1);
-n2_child_size = virXMLChildElementCount(n2);
-if (n1_child_size < 0 || n2_child_size < 0 ||
-n1_child_size < n2_child_size)
-return false;
-
-if (n1_child_size == 0 && n2_child_size == 0)
-return true;
-
-bitmap = virBitmapNew(n1_child_size);
-
-child2 = n2->children;
-while (child2) {
-if (child2->type != XML_ELEMENT_NODE) {
-child2 = child2->next;
-continue;
-}
-
-child1 = n1->children;
-n1_iter = 0;
-found = false;
-while (child1) {
-if (child1->type != XML_ELEMENT_NODE) {
-child1 = child1->next;
-continue;
-}
-
-if (virBitmapGetBit(bitmap, n1_iter, ) < 0) {
-vshError(NULL, "%s", _("Bad child elements counting."));
-goto cleanup;
-}
-
-if (visited) {
-child1 = child1->next;
-n1_iter++;
-continue;
-}
-
-if (xmlStrEqual(child1->name, child2->name)) {
-found = true;
-if (virBitmapSetBit(bitmap, n1_iter) < 0) {
-vshError(NULL, "%s", _("Bad child elements counting."));
-goto cleanup;
-}
-
-if (!virshNodeIsSuperset(child1, child2))
-goto cleanup;
-
-break;
-}
-
-child1 = child1->next;
-n1_iter++;
-}
-
-if (!found)
-goto cleanup;
-
-child2 = child2->next;
-}
-
-ret = true;
-
- cleanup:
-virBitmapFree(bitmap);
-return ret;
-}
-
-
 /*
  * "detach-device" command
  */
-- 
2.25.3

[PATCH v3] qemuMonitorJSONCheckReply: Use g_autofree

[Yi Li]

Eliminate cleanup code by using g_autofree.

Signed-off-by: Yi Li 
---
 src/qemu/qemu_monitor_json.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 6c763ecc12..4994ace071 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -455,8 +455,8 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
 
 data = virJSONValueObjectGet(reply, "return");
 if (virJSONValueGetType(data) != type) {
-char *cmdstr = virJSONValueToString(cmd, false);
-char *retstr = virJSONValueToString(data, false);
+g_autofree char *cmdstr = virJSONValueToString(cmd, false);
+g_autofree char *retstr = virJSONValueToString(data, false);
 
 VIR_DEBUG("Unexpected return type %d (expecting %d) for command %s: 
%s",
   virJSONValueGetType(data), type, cmdstr, retstr);
@@ -464,8 +464,6 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
_("unexpected type returned by QEMU command '%s'"),
qemuMonitorJSONCommandName(cmd));
 
-VIR_FREE(cmdstr);
-VIR_FREE(retstr);
 return -1;
 }
 
-- 
2.25.3

[PATCH v2] qemu_monitor_json.c: use g_autofree

[Yi Li]

Eliminate cleanup code by using g_autofree in qemuMonitorJSONCheckErrorFull()
and qemuMonitorJSONCheckReply

Signed-off-by: Yi Li 
---
 src/qemu/qemu_monitor_json.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 26ac499fc5..f76b369191 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -392,10 +392,11 @@ qemuMonitorJSONCheckErrorFull(virJSONValuePtr cmd,
   virJSONValuePtr reply,
   bool report)
 {
+g_autofree char *cmdstr = virJSONValueToString(cmd, false);
+g_autofree char *replystr = virJSONValueToString(reply, false);
+
 if (virJSONValueObjectHasKey(reply, "error")) {
 virJSONValuePtr error = virJSONValueObjectGet(reply, "error");
-g_autofree char *cmdstr = virJSONValueToString(cmd, false);
-g_autofree char *replystr = virJSONValueToString(reply, false);
 
 /* Log the full JSON formatted command & error */
 VIR_DEBUG("unable to execute QEMU command %s: %s",
@@ -417,8 +418,6 @@ qemuMonitorJSONCheckErrorFull(virJSONValuePtr cmd,
 
 return -1;
 } else if (!virJSONValueObjectHasKey(reply, "return")) {
-g_autofree char *cmdstr = virJSONValueToString(cmd, false);
-g_autofree char *replystr = virJSONValueToString(reply, false);
 
 VIR_DEBUG("Neither 'return' nor 'error' is set in the JSON reply %s: 
%s",
   NULLSTR(cmdstr), NULLSTR(replystr));
@@ -455,8 +454,8 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
 
 data = virJSONValueObjectGet(reply, "return");
 if (virJSONValueGetType(data) != type) {
-char *cmdstr = virJSONValueToString(cmd, false);
-char *retstr = virJSONValueToString(data, false);
+g_autofree char *cmdstr = virJSONValueToString(cmd, false);
+g_autofree char *retstr = virJSONValueToString(data, false);
 
 VIR_DEBUG("Unexpected return type %d (expecting %d) for command %s: 
%s",
   virJSONValueGetType(data), type, cmdstr, retstr);
@@ -464,8 +463,6 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
_("unexpected type returned by QEMU command '%s'"),
qemuMonitorJSONCommandName(cmd));
 
-VIR_FREE(cmdstr);
-VIR_FREE(retstr);
 return -1;
 }
 
-- 
2.25.3

[PATCH] qemuMonitorJSONCheckReply: Use g_autofree

[Yi Li]

Eliminate cleanup code by using g_autofree.

Signed-off-by: Yi Li 
---
 src/qemu/qemu_monitor_json.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 26ac499fc5..f76b369191 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -392,10 +392,11 @@ qemuMonitorJSONCheckErrorFull(virJSONValuePtr cmd,
   virJSONValuePtr reply,
   bool report)
 {
+g_autofree char *cmdstr = virJSONValueToString(cmd, false);
+g_autofree char *replystr = virJSONValueToString(reply, false);
+
 if (virJSONValueObjectHasKey(reply, "error")) {
 virJSONValuePtr error = virJSONValueObjectGet(reply, "error");
-g_autofree char *cmdstr = virJSONValueToString(cmd, false);
-g_autofree char *replystr = virJSONValueToString(reply, false);
 
 /* Log the full JSON formatted command & error */
 VIR_DEBUG("unable to execute QEMU command %s: %s",
@@ -417,8 +418,6 @@ qemuMonitorJSONCheckErrorFull(virJSONValuePtr cmd,
 
 return -1;
 } else if (!virJSONValueObjectHasKey(reply, "return")) {
-g_autofree char *cmdstr = virJSONValueToString(cmd, false);
-g_autofree char *replystr = virJSONValueToString(reply, false);
 
 VIR_DEBUG("Neither 'return' nor 'error' is set in the JSON reply %s: 
%s",
   NULLSTR(cmdstr), NULLSTR(replystr));
@@ -455,8 +454,8 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
 
 data = virJSONValueObjectGet(reply, "return");
 if (virJSONValueGetType(data) != type) {
-char *cmdstr = virJSONValueToString(cmd, false);
-char *retstr = virJSONValueToString(data, false);
+g_autofree char *cmdstr = virJSONValueToString(cmd, false);
+g_autofree char *retstr = virJSONValueToString(data, false);
 
 VIR_DEBUG("Unexpected return type %d (expecting %d) for command %s: 
%s",
   virJSONValueGetType(data), type, cmdstr, retstr);
@@ -464,8 +463,6 @@ qemuMonitorJSONCheckReply(virJSONValuePtr cmd,
_("unexpected type returned by QEMU command '%s'"),
qemuMonitorJSONCommandName(cmd));
 
-VIR_FREE(cmdstr);
-VIR_FREE(retstr);
 return -1;
 }
 
-- 
2.25.3

[PATCH] Remove redundant check when storage pool is mounted

[Yi Li]

virFileComparePaths just return 0 or 1 after commit 7b48bb8
so break while after virFileComparePaths return 1

Signed-off-by: Yi Li 
---
 src/storage/storage_backend_fs.c | 8 ++--
 src/util/virfile.c   | 1 -
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 536e5cf952..30c2367df4 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -245,7 +245,6 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr 
pool)
 FILE *mtab;
 struct mntent ent;
 char buf[1024];
-int rc1, rc2;
 g_autofree char *src = NULL;
 
 if ((mtab = fopen(_PATH_MOUNTED, "r")) == NULL) {
@@ -262,11 +261,8 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr 
pool)
 /* compare both mount destinations and sources to be sure the mounted
  * FS pool is really the one we're looking for
  */
-if ((rc1 = virFileComparePaths(ent.mnt_dir, def->target.path)) < 0 ||
-(rc2 = virFileComparePaths(ent.mnt_fsname, src)) < 0)
-goto cleanup;
-
-if (rc1 && rc2) {
+if (virFileComparePaths(ent.mnt_dir, def->target.path) &&
+virFileComparePaths(ent.mnt_fsname, src)) {
 ret = 1;
 goto cleanup;
 }
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 61d2c16072..e120d277d0 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3884,7 +3884,6 @@ virFileCopyACLs(const char *src,
  * Returns:
  *  1 : Equal
  *  0 : Non-Equal
- * -1 : Error
  */
 int
 virFileComparePaths(const char *p1, const char *p2)
-- 
2.25.3

[PATCH] tools: Remove unnecessary condition check

[Yi Li]

virshDomainFree correctly handle null pointer parameters.
There is no need to check if the parameter is null before
calling this function.

Signed-off-by: Yi Li 
---
 tools/virsh-domain-monitor.c | 6 ++
 tools/virsh-domain.c | 3 +--
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 016885..4c0da3c695 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1610,10 +1610,8 @@ virshDomainListFree(virshDomainListPtr domlist)
 size_t i;
 
 if (domlist && domlist->domains) {
-for (i = 0; i < domlist->ndomains; i++) {
-if (domlist->domains[i])
-virshDomainFree(domlist->domains[i]);
-}
+for (i = 0; i < domlist->ndomains; i++)
+virshDomainFree(domlist->domains[i]);
 VIR_FREE(domlist->domains);
 }
 VIR_FREE(domlist);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index d1d3f8e566..08496cc5bf 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -5509,8 +5509,7 @@ doDump(void *opaque)
 pthread_sigmask(SIG_SETMASK, , NULL);
  out_sig:
 #endif /* !WIN32 */
-if (dom)
-virshDomainFree(dom);
+virshDomainFree(dom);
 g_main_loop_quit(data->eventLoop);
 }
 
-- 
2.25.3

[PATCH] conf: remove unused variable

[Yi Li]

Signed-off-by: Yi Li 
---
 src/conf/snapshot_conf.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 857d04c476..f73eeb06c7 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -231,7 +231,7 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
 xmlNodePtr inactiveDomNode = NULL;
 size_t i;
 int n;
-char *creation = NULL, *state = NULL;
+char *state = NULL;
 int active;
 char *tmp;
 char *memorySnapshot = NULL;
@@ -404,7 +404,6 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
 ret = g_steal_pointer();
 
  cleanup:
-VIR_FREE(creation);
 VIR_FREE(state);
 VIR_FREE(nodes);
 VIR_FREE(memorySnapshot);
-- 
2.25.3

[PATCH v2] Unlock the storage pool objects after looking it up

[Yi Li]

Use g_new0 to allocate and remove NULL checks from callers
and the lock will release properly

Signed-off-by: Yi Li 
---
 src/conf/virstorageobj.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 13b75b648d..f3c54d0c52 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -1036,10 +1036,7 @@ virStoragePoolObjVolumeListExport(virConnectPtr conn,
 return ret;
 }
 
-if (VIR_ALLOC_N(data.vols, virHashSize(volumes->objsName) + 1) < 0) {
-virObjectRWUnlock(volumes);
-return -1;
-}
+data.vols = g_new0(virStorageVolPtr, virHashSize(volumes->objsName) + 1);
 
 virHashForEach(volumes->objsName, 
virStoragePoolObjVolumeListExportCallback, );
 virObjectRWUnlock(volumes);
@@ -2077,8 +2074,13 @@ virStoragePoolObjListExport(virConnectPtr conn,
 
 virObjectRWLockRead(poolobjs);
 
-if (pools && VIR_ALLOC_N(data.pools, virHashSize(poolobjs->objs) + 1) < 0)
-goto error;
+if (!pools) {
+int ret = virHashSize(poolobjs->objs);
+virObjectRWUnlock(poolobjs);
+return ret;
+}
+
+data.pools = g_new0(virStoragePoolPtr, virHashSize(poolobjs->objs) + 1);
 
 virHashForEach(poolobjs->objs, virStoragePoolObjListExportCallback, );
 virObjectRWUnlock(poolobjs);
-- 
2.25.3

[PATCH] Unlock the storage pool objects after looking it up

[Yi Li]

The lock should be released.

Signed-off-by: Yi Li 
---
 src/conf/virstorageobj.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 13b75b648d..9f24ae67ca 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -2077,8 +2077,10 @@ virStoragePoolObjListExport(virConnectPtr conn,
 
 virObjectRWLockRead(poolobjs);
 
-if (pools && VIR_ALLOC_N(data.pools, virHashSize(poolobjs->objs) + 1) < 0)
-goto error;
+if (pools && VIR_ALLOC_N(data.pools, virHashSize(poolobjs->objs) + 1) < 0) 
{
+virObjectRWUnlock(poolobjs);
+return -1;
+}
 
 virHashForEach(poolobjs->objs, virStoragePoolObjListExportCallback, );
 virObjectRWUnlock(poolobjs);
-- 
2.25.3

[PATCH] storage: use local dom_disk parameter

[Yi Li]

replace vm->def->disks[i]  with dom_disk parameter

Signed-off-by: Yi Li 
---
 src/qemu/qemu_driver.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index daa3cb397d..27f43124ac 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14841,14 +14841,13 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm,
   active) < 0)
 return -1;
 
-if (vm->def->disks[i]->src->format > 0 &&
-vm->def->disks[i]->src->format != VIR_STORAGE_FILE_QCOW2) {
+if (dom_disk->src->format > 0 &&
+dom_disk->src->format != VIR_STORAGE_FILE_QCOW2) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("internal snapshot for disk %s unsupported "
  "for storage type %s"),
disk->name,
-   virStorageFileFormatTypeToString(
-   vm->def->disks[i]->src->format));
+   
virStorageFileFormatTypeToString(dom_disk->src->format));
 return -1;
 }
 break;
-- 
2.13.6

[libvirt] [PATCH v3 3/3] Storage: Use errno parameter in virReportSystemError

[Yi Li]

Use errno parameter in virReportSystemError.
Remove hold function return values if don't need.

Signed-off-by: Yi Li 
---
 src/storage/storage_backend_rbd.c | 159 ++
 1 file changed, 76 insertions(+), 83 deletions(-)

diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index 5f14156..78a8e95 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -184,7 +184,7 @@ static int
 virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
   virStoragePoolDefPtr def)
 {
-int rc, ret = -1;
+int ret = -1;
 virStoragePoolSourcePtr source = >source;
 virStorageAuthDefPtr authdef = source->auth;
 unsigned char *secret_value = NULL;
@@ -202,8 +202,8 @@ 
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
 if (authdef) {
 VIR_DEBUG("Using cephx authorization, username: %s", 
authdef->username);
 
-if ((rc = rados_create(>cluster, authdef->username)) < 0) {
-virReportSystemError(-rc, "%s", _("failed to initialize RADOS"));
+if (rados_create(>cluster, authdef->username) < 0) {
+virReportSystemError(errno, "%s", _("failed to initialize RADOS"));
 goto cleanup;
 }
 
@@ -318,8 +318,8 @@ 
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
 }
 
 ptr->starttime = time(0);
-if ((rc = rados_connect(ptr->cluster)) < 0) {
-virReportSystemError(-rc, _("failed to connect to the RADOS monitor 
on: %s"),
+if (rados_connect(ptr->cluster) < 0) {
+virReportSystemError(errno, _("failed to connect to the RADOS monitor 
on: %s"),
  mon_buff);
 goto cleanup;
 }
@@ -341,7 +341,7 @@ virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr 
ptr,
 virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
 int rc = rados_ioctx_create(ptr->cluster, def->source.name, >ioctx);
 if (rc < 0) {
-virReportSystemError(-rc, _("failed to create the RBD IoCTX. Does the 
pool '%s' exist?"),
+virReportSystemError(errno, _("failed to create the RBD IoCTX. Does 
the pool '%s' exist?"),
  def->source.name);
 }
 return rc;
@@ -410,7 +410,7 @@ volStorageBackendRBDGetFeatures(rbd_image_t image,
 int rc;
 
 if ((rc = rbd_get_features(image, features)) < 0) {
-virReportSystemError(-rc, _("failed to get the features of RBD image "
+virReportSystemError(errno, _("failed to get the features of RBD image 
"
  "%s"), volname);
 return rc;
 }
@@ -427,7 +427,7 @@ volStorageBackendRBDGetFlags(rbd_image_t image,
 int rc;
 
 if ((rc = rbd_get_flags(image, flags)) < 0) {
-virReportSystemError(-rc,
+virReportSystemError(errno,
  _("failed to get the flags of RBD image %s"),
  volname);
 return rc;
@@ -467,7 +467,7 @@ virStorageBackendRBDSetAllocation(virStorageVolDefPtr vol,
 if ((rc = rbd_diff_iterate2(image, NULL, 0, info->size, 0, 1,
,
)) < 0) {
-virReportSystemError(-rc, _("failed to iterate RBD image '%s'"),
+virReportSystemError(errno, _("failed to iterate RBD image '%s'"),
  vol->name);
 return rc;
 }
@@ -520,14 +520,14 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr 
vol,
 
 if ((rc = rbd_open_read_only(ptr->ioctx, vol->name, , NULL)) < 0) {
 ret = rc;
-virReportSystemError(-rc, _("failed to open the RBD image '%s'"),
+virReportSystemError(errno, _("failed to open the RBD image '%s'"),
  vol->name);
 goto cleanup;
 }
 
 if ((rc = rbd_stat(image, , sizeof(info))) < 0) {
 ret = rc;
-virReportSystemError(-rc, _("failed to stat the RBD image '%s'"),
+virReportSystemError(errno, _("failed to stat the RBD image '%s'"),
  vol->name);
 goto cleanup;
 }
@@ -600,7 +600,7 @@ 
virStorageBackendRBDGetVolNames(virStorageBackendRBDStatePtr ptr)
 if (rc >= 0)
 break;
 if (rc != -ERANGE) {
-virReportSystemError(-rc, "%s", _("Unable to list RBD images"));
+virReportSystemError(errno, "%s", _("Unable to list RBD images"));
 goto error;
 }
 }
@@ -641,7 +641,7 @@ 
virStorageBackendRBDGetVolNames(virStorageBackendRBDStatePtr ptr)
 if (rc >= 0)
 break;
 

[libvirt] [PATCH v3 1/3] Storage: Use rc hold intermediate function return values.

[Yi Li]

most libvirt code uses 'int rc' to hold intermediate
function return values. consistent with the rest of libvirt.

Signed-off-by: Yi Li 
---
 src/storage/storage_backend_rbd.c | 202 ++
 1 file changed, 96 insertions(+), 106 deletions(-)

diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index 34088f7..81f7cd5 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -184,8 +184,7 @@ static int
 virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
   virStoragePoolDefPtr def)
 {
-int ret = -1;
-int r = 0;
+int rc, ret = -1;
 virStoragePoolSourcePtr source = >source;
 virStorageAuthDefPtr authdef = source->auth;
 unsigned char *secret_value = NULL;
@@ -203,8 +202,8 @@ 
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
 if (authdef) {
 VIR_DEBUG("Using cephx authorization, username: %s", 
authdef->username);
 
-if ((r = rados_create(>cluster, authdef->username)) < 0) {
-virReportSystemError(-r, "%s", _("failed to initialize RADOS"));
+if ((rc = rados_create(>cluster, authdef->username)) < 0) {
+virReportSystemError(-rc, "%s", _("failed to initialize RADOS"));
 goto cleanup;
 }
 
@@ -319,8 +318,8 @@ 
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
 }
 
 ptr->starttime = time(0);
-if ((r = rados_connect(ptr->cluster)) < 0) {
-virReportSystemError(-r, _("failed to connect to the RADOS monitor on: 
%s"),
+if ((rc = rados_connect(ptr->cluster)) < 0) {
+virReportSystemError(-rc, _("failed to connect to the RADOS monitor 
on: %s"),
  mon_buff);
 goto cleanup;
 }
@@ -340,12 +339,12 @@ 
virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr,
   virStoragePoolObjPtr pool)
 {
 virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
-int r = rados_ioctx_create(ptr->cluster, def->source.name, >ioctx);
-if (r < 0) {
-virReportSystemError(-r, _("failed to create the RBD IoCTX. Does the 
pool '%s' exist?"),
+int rc = rados_ioctx_create(ptr->cluster, def->source.name, >ioctx);
+if (rc < 0) {
+virReportSystemError(-rc, _("failed to create the RBD IoCTX. Does the 
pool '%s' exist?"),
  def->source.name);
 }
-return r;
+return rc;
 }
 
 static void
@@ -408,10 +407,10 @@ volStorageBackendRBDGetFeatures(rbd_image_t image,
 const char *volname,
 uint64_t *features)
 {
-int r;
+int rc;
 
-if ((r = rbd_get_features(image, features)) < 0) {
-virReportSystemError(-r, _("failed to get the features of RBD image "
+if ((rc = rbd_get_features(image, features)) < 0) {
+virReportSystemError(-rc, _("failed to get the features of RBD image "
  "%s"), volname);
 return -1;
 }
@@ -462,13 +461,13 @@ virStorageBackendRBDSetAllocation(virStorageVolDefPtr vol,
   rbd_image_t *image,
   rbd_image_info_t *info)
 {
-int r;
+int rc;
 size_t allocation = 0;
 
-if ((r = rbd_diff_iterate2(image, NULL, 0, info->size, 0, 1,
+if ((rc = rbd_diff_iterate2(image, NULL, 0, info->size, 0, 1,
,
)) < 0) {
-virReportSystemError(-r, _("failed to iterate RBD image '%s'"),
+virReportSystemError(-rc, _("failed to iterate RBD image '%s'"),
  vol->name);
 return -1;
 }
@@ -512,24 +511,23 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr 
vol,
virStoragePoolObjPtr pool,
virStorageBackendRBDStatePtr ptr)
 {
-int ret = -1;
+int rc, ret = -1;
 virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
-int r = 0;
 rbd_image_t image = NULL;
 rbd_image_info_t info;
 uint64_t features;
 uint64_t flags;
 
-if ((r = rbd_open_read_only(ptr->ioctx, vol->name, , NULL)) < 0) {
-ret = -r;
-virReportSystemError(-r, _("failed to open the RBD image '%s'"),
+if ((rc = rbd_open_read_only(ptr->ioctx, vol->name, , NULL)) < 0) {
+ret = -rc;
+virReportSystemError(-rc, _("failed to open the RBD image '%s'"),
  vol->name);
 goto cleanup;
 }
 
-if ((r = rbd_stat(image, , sizeof(info))) < 0) {
-ret = -r;
-virRepo

[libvirt] [PATCH v3 2/3] storage: Fix volStorageBackendRBDRefreshVolInfo function return errors

[Yi Li]

Fix the return value status comparison checking for call to
volStorageBackendRBDRefreshVolInfo introduced by commit id f46d137e.

we only should fail when the return is < 0. -ENOENT, -ETIMEDOUT will
ignore according commit id f46d137e.

Signed-off-by: Yi Li 
---
 src/storage/storage_backend_rbd.c | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index 81f7cd5..5f14156 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -412,7 +412,7 @@ volStorageBackendRBDGetFeatures(rbd_image_t image,
 if ((rc = rbd_get_features(image, features)) < 0) {
 virReportSystemError(-rc, _("failed to get the features of RBD image "
  "%s"), volname);
-return -1;
+return rc;
 }
 
 return 0;
@@ -430,7 +430,7 @@ volStorageBackendRBDGetFlags(rbd_image_t image,
 virReportSystemError(-rc,
  _("failed to get the flags of RBD image %s"),
  volname);
-return -1;
+return rc;
 }
 
 return 0;
@@ -469,7 +469,7 @@ virStorageBackendRBDSetAllocation(virStorageVolDefPtr vol,
)) < 0) {
 virReportSystemError(-rc, _("failed to iterate RBD image '%s'"),
  vol->name);
-return -1;
+return rc;
 }
 
 VIR_DEBUG("Found %zu bytes allocated for RBD image %s",
@@ -519,24 +519,28 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr 
vol,
 uint64_t flags;
 
 if ((rc = rbd_open_read_only(ptr->ioctx, vol->name, , NULL)) < 0) {
-ret = -rc;
+ret = rc;
 virReportSystemError(-rc, _("failed to open the RBD image '%s'"),
  vol->name);
 goto cleanup;
 }
 
 if ((rc = rbd_stat(image, , sizeof(info))) < 0) {
-ret = -rc;
+ret = rc;
 virReportSystemError(-rc, _("failed to stat the RBD image '%s'"),
  vol->name);
 goto cleanup;
 }
 
-if (volStorageBackendRBDGetFeatures(image, vol->name, ) < 0)
+if ((rc = volStorageBackendRBDGetFeatures(image, vol->name, )) < 
0) {
+ret = rc;
 goto cleanup;
+}
 
-if (volStorageBackendRBDGetFlags(image, vol->name, ) < 0)
+if ((rc = volStorageBackendRBDGetFlags(image, vol->name, )) < 0) {
+ret = rc;
 goto cleanup;
+}
 
 vol->target.capacity = info.size;
 vol->type = VIR_STORAGE_VOL_NETWORK;
@@ -549,8 +553,10 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol,
   "Querying for actual allocation",
   def->source.name, vol->name);
 
-if (virStorageBackendRBDSetAllocation(vol, image, ) < 0)
+if ((rc = virStorageBackendRBDSetAllocation(vol, image, )) < 0) {
+ret = rc;
 goto cleanup;
+   }
 } else {
 vol->target.allocation = info.obj_size * info.num_objs;
 }
-- 
2.7.5




--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2] storage: Fix daemon crash on lookup storagepool by targetpath

[Yi Li]

Causing a crash when storagePoolLookupByTargetPath beacuse of
Some types of storage pool have no target elements.
Use STREQ_NULLABLE instead of STREQ
Avoids segfaults when using NULL arguments.

Core was generated by `/usr/sbin/libvirtd'.
Program terminated with signal 11, Segmentation fault.
(gdb) bt
0  0x9e951388 in strcmp () from /lib64/libc.so.6
1  0x92103e9c in storagePoolLookupByTargetPathCallback (
obj=0x7009aab0, opaque=0x801058b0) at storage/storage_driver.c:1649
2  0x9f2c52a4 in virStoragePoolObjListSearchCb (
payload=0x801058b0, name=, opaque=)
at conf/virstorageobj.c:476
3  0x9f1f2f7c in virHashSearch (ctable=0x800f4f60,
iter=iter@entry=0x9f2c5278 ,
data=data@entry=0x95af7488, name=name@entry=0x0) at util/virhash.c:696
4  0x9f2c64f0 in virStoragePoolObjListSearch (pools=0x800f2ce0,
searcher=searcher@entry=0x92103e68 
,
 opaque=) at conf/virstorageobj.c:505
5  0x92101f54 in storagePoolLookupByTargetPath (conn=0x5c0009f0,
path=0x7009a850 "/vms/images") at storage/storage_driver.c:1672

Signed-off-by: Yi Li 
---
 src/storage/storage_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index a33328d..72ba252 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1713,7 +1713,7 @@ 
storagePoolLookupByTargetPathCallback(virStoragePoolObjPtr obj,
 return false;
 
 def = virStoragePoolObjGetDef(obj);
-return STREQ(path, def->target.path);
+return STREQ_NULLABLE(path, def->target.path);
 }
 
 
-- 
2.7.5




--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] storage: Fix daemon crash on lookup storagepool by targetpath

[Yi Li]

Causing a crash when storagePoolLookupByTargetPath because of
Some types of storage pool have no target elements.

Core was generated by `/usr/sbin/libvirtd'.
Program terminated with signal 11, Segmentation fault.
(gdb) bt
0  0x9e951388 in strcmp () from /lib64/libc.so.6
1  0x92103e9c in storagePoolLookupByTargetPathCallback (
obj=0x7009aab0, opaque=0x801058b0) at storage/storage_driver.c:1649
2  0x9f2c52a4 in virStoragePoolObjListSearchCb (
payload=0x801058b0, name=, opaque=)
at conf/virstorageobj.c:476
3  0x9f1f2f7c in virHashSearch (ctable=0x800f4f60,
iter=iter@entry=0x9f2c5278 ,
data=data@entry=0x95af7488, name=name@entry=0x0) at util/virhash.c:696
4  0x9f2c64f0 in virStoragePoolObjListSearch (pools=0x800f2ce0,
searcher=searcher@entry=0x92103e68 
,
 opaque=) at conf/virstorageobj.c:505
5  0x92101f54 in storagePoolLookupByTargetPath (conn=0x5c0009f0,
path=0x7009a850 "/vms/images") at storage/storage_driver.c:1672

Signed-off-by: Yi Li 
---
 src/storage/storage_driver.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index a33328d..5863e1f 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1713,6 +1713,12 @@ 
storagePoolLookupByTargetPathCallback(virStoragePoolObjPtr obj,
 return false;
 
 def = virStoragePoolObjGetDef(obj);
+
+/* Some Storage pool types have no Target elements,
+ * so we should check it. */
+if (def->target.path == NULL)
+return false;
+
 return STREQ(path, def->target.path);
 }
 
-- 
2.7.5




--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2] storage: Fix volStorageBackendRBDRefreshVolInfo

[Yi Li]

Fix the return value status comparison checking for call to
volStorageBackendRBDRefreshVolInfo introduced by commit id f46d137e.

we only should fail when the return is < 0. -ENOENT, -ETIMEDOUT will
ignore according commit id f46d137e.

Signed-off-by: Yi Li 
---
 src/storage/storage_backend_rbd.c | 32 ++--
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index 7ce26ed..f7319d6 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -411,6 +411,7 @@ volStorageBackendRBDGetFeatures(rbd_image_t image,
 int r, ret = -1;
 
 if ((r = rbd_get_features(image, features)) < 0) {
+ret = r;
 virReportSystemError(-r, _("failed to get the features of RBD image "
  "%s"), volname);
 goto cleanup;
@@ -427,16 +428,19 @@ volStorageBackendRBDGetFlags(rbd_image_t image,
  const char *volname,
  uint64_t *flags)
 {
-int rc;
+int r, ret = -1;
 
-if ((rc = rbd_get_flags(image, flags)) < 0) {
-virReportSystemError(-rc,
+if ((r = rbd_get_flags(image, flags)) < 0) {
+ret = r;
+virReportSystemError(-r,
  _("failed to get the flags of RBD image %s"),
  volname);
-return -1;
+goto cleanup;
 }
+ret = 0;
 
-return 0;
+ cleanup:
+return ret;
 }
 
 static bool
@@ -470,6 +474,7 @@ virStorageBackendRBDSetAllocation(virStorageVolDefPtr vol,
 if ((r = rbd_diff_iterate2(image, NULL, 0, info->size, 0, 1,
,
)) < 0) {
+ret = r;
 virReportSystemError(-r, _("failed to iterate RBD image '%s'"),
  vol->name);
 goto cleanup;
@@ -525,24 +530,28 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr 
vol,
 uint64_t flags;
 
 if ((r = rbd_open_read_only(ptr->ioctx, vol->name, , NULL)) < 0) {
-ret = -r;
+ret = r;
 virReportSystemError(-r, _("failed to open the RBD image '%s'"),
  vol->name);
 goto cleanup;
 }
 
 if ((r = rbd_stat(image, , sizeof(info))) < 0) {
-ret = -r;
+ret = r;
 virReportSystemError(-r, _("failed to stat the RBD image '%s'"),
  vol->name);
 goto cleanup;
 }
 
-if (volStorageBackendRBDGetFeatures(image, vol->name, ) < 0)
+if ((r = volStorageBackendRBDGetFeatures(image, vol->name, )) < 
0) {
+ret = r;
 goto cleanup;
+}
 
-if (volStorageBackendRBDGetFlags(image, vol->name, ) < 0)
+if ((r = volStorageBackendRBDGetFlags(image, vol->name, )) < 0) {
+ret = r;
 goto cleanup;
+}
 
 vol->target.capacity = info.size;
 vol->type = VIR_STORAGE_VOL_NETWORK;
@@ -555,8 +564,11 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol,
   "Querying for actual allocation",
   def->source.name, vol->name);
 
-if (virStorageBackendRBDSetAllocation(vol, image, ) < 0)
+if ((r = virStorageBackendRBDSetAllocation(vol, image, ) < 0)) {
+ret = r;
 goto cleanup;
+}
+
 } else {
 vol->target.allocation = info.obj_size * info.num_objs;
 }
-- 
2.7.5



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] storage: Fix volStorageBackendRBDRefreshVolInfo

[Yi Li]

Fix the return value status comparison checking for call to
volStorageBackendRBDRefreshVolInfo introduced by commit id f46d137e.

we only should fail when the return is < 0. -ENOENT, -ETIMEDOUT will
ignore according commit id f46d137e.

Signed-off-by: Yi Li 
---
 src/storage/storage_backend_rbd.c | 40 ---
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index 7ce26ed..1551cf4 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -411,6 +411,7 @@ volStorageBackendRBDGetFeatures(rbd_image_t image,
 int r, ret = -1;
 
 if ((r = rbd_get_features(image, features)) < 0) {
+ret = r;
 virReportSystemError(-r, _("failed to get the features of RBD image "
  "%s"), volname);
 goto cleanup;
@@ -427,16 +428,19 @@ volStorageBackendRBDGetFlags(rbd_image_t image,
  const char *volname,
  uint64_t *flags)
 {
-int rc;
+int r, ret = -1;
 
-if ((rc = rbd_get_flags(image, flags)) < 0) {
-virReportSystemError(-rc,
+if ((r = rbd_get_flags(image, flags)) < 0) {
+ret = r;
+virReportSystemError(-r,
  _("failed to get the flags of RBD image %s"),
  volname);
-return -1;
+goto cleanup;
 }
+ret = 0;
 
-return 0;
+ cleanup:
+return ret;
 }
 
 static bool
@@ -470,6 +474,7 @@ virStorageBackendRBDSetAllocation(virStorageVolDefPtr vol,
 if ((r = rbd_diff_iterate2(image, NULL, 0, info->size, 0, 1,
,
)) < 0) {
+ret = r;
 virReportSystemError(-r, _("failed to iterate RBD image '%s'"),
  vol->name);
 goto cleanup;
@@ -525,24 +530,28 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr 
vol,
 uint64_t flags;
 
 if ((r = rbd_open_read_only(ptr->ioctx, vol->name, , NULL)) < 0) {
-ret = -r;
+ret = r;
 virReportSystemError(-r, _("failed to open the RBD image '%s'"),
  vol->name);
 goto cleanup;
 }
 
 if ((r = rbd_stat(image, , sizeof(info))) < 0) {
-ret = -r;
+ret = r;
 virReportSystemError(-r, _("failed to stat the RBD image '%s'"),
  vol->name);
 goto cleanup;
 }
 
-if (volStorageBackendRBDGetFeatures(image, vol->name, ) < 0)
+if ((r = volStorageBackendRBDGetFeatures(image, vol->name, )) < 
0) {
+ret = r;
 goto cleanup;
+}
 
-if (volStorageBackendRBDGetFlags(image, vol->name, ) < 0)
+if ((r = volStorageBackendRBDGetFlags(image, vol->name, )) < 0) {
+ret = r;
 goto cleanup;
+}
 
 vol->target.capacity = info.size;
 vol->type = VIR_STORAGE_VOL_NETWORK;
@@ -555,8 +564,11 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol,
   "Querying for actual allocation",
   def->source.name, vol->name);
 
-if (virStorageBackendRBDSetAllocation(vol, image, ) < 0)
+if ((r = virStorageBackendRBDSetAllocation(vol, image, ) < 0)) {
+ret = r;
 goto cleanup;
+}
+
 } else {
 vol->target.allocation = info.obj_size * info.num_objs;
 }
@@ -734,12 +746,10 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool)
  *
  * Do not error out and simply ignore the volume
  */
-if (r < 0) {
-if (r == -ENOENT || r == -ETIMEDOUT)
-continue;
-
+if (r == -ENOENT || r == -ETIMEDOUT)
+continue;
+else if (r < 0)
 goto cleanup;
-}
 
 if (virStoragePoolObjAddVol(pool, vol) < 0)
 goto cleanup;
-- 
2.7.5



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2] storage: improve the while loop virStorageBackendFileSystemIsMounted

[Yi Li]

Move virStorageBackendFileSystemGetPoolSource outside of the while loop

Signed-off-by: Yi Li 
---
 src/storage/storage_backend_fs.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 02b8248..92516c8 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -258,10 +258,10 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr 
pool)
 goto cleanup;
 }
 
-while ((getmntent_r(mtab, , buf, sizeof(buf))) != NULL) {
-if (!(src = virStorageBackendFileSystemGetPoolSource(pool)))
-goto cleanup;
+if ((src = virStorageBackendFileSystemGetPoolSource(pool)) == NULL)
+goto cleanup;
 
+while ((getmntent_r(mtab, , buf, sizeof(buf))) != NULL) {
 /* compare both mount destinations and sources to be sure the mounted
  * FS pool is really the one we're looking for
  */
@@ -274,12 +274,13 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr 
pool)
 goto cleanup;
 }
 
-VIR_FREE(src);
 }
 
 ret = 0;
 
  cleanup:
+if (src)
+VIR_FREE(src);
 VIR_FORCE_FCLOSE(mtab);
 return ret;
 }
-- 
2.7.5



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] storage: improve the while loop virStorageBackendFileSystemIsMounted

[Yi Li]

Move virStorageBackendFileSystemGetPoolSource outside of the while loop

Signed-off-by: Yi Li 
---
 src/storage/storage_backend_fs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 02b8248..10d9457 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -258,10 +258,10 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr 
pool)
 goto cleanup;
 }
 
-while ((getmntent_r(mtab, , buf, sizeof(buf))) != NULL) {
-if (!(src = virStorageBackendFileSystemGetPoolSource(pool)))
-goto cleanup;
+if ((src = virStorageBackendFileSystemGetPoolSource(pool)) == NULL)
+goto cleanup;
 
+while ((getmntent_r(mtab, , buf, sizeof(buf))) != NULL) {
 /* compare both mount destinations and sources to be sure the mounted
  * FS pool is really the one we're looking for
  */
-- 
2.7.5



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: storage: drop VIR_STORAGE_FILE_AUTO_SAFE

[Yi Li]

>> merge VIR_STORAGE_FILE_AUTO_SAFE/VIR_STORAGE_FILE_AUTO to 
>> VIR_STORAGE_FILE_AUTO
>> virStorageFileProbeFormatFromBuf will probe the backingStore format.
>>
>> Fix the booting issue when setting backingStore format (QCOW image) to RAW 
>> image.
>
>This description does not really describe what the problem is.
>

The Guest VM cann't boot correctly as below:
1: Guest VM disk info when shutdown 
..
/usr/libexec/qemu-kvm

  
  
  
  
  

..
pls:
[root@***-130 ~]# qemu-img info 
/vms/images/.transient/f6e5eb8b-7d81-443d-aab0-bb1cca1cf29e
image: /vms/images/.transient/f6e5eb8b-7d81-443d-aab0-bb1cca1cf29e
file format: qcow2
virtual size: 80G (85899345920 bytes)
disk size: 15M
cluster_size: 65536
backing file: /vms/images/f6e5eb8b-7d81-443d-aab0-bb1cca1cf29e
Format specific information:
compat: 1.1
lazy refcounts: false
refcount bits: 16
corrupt: false
[root@***-130 ~]# qemu-img info /vms/images/f6e5eb8b-7d81-443d-aab0-bb1cca1cf29e
image: /vms/images/f6e5eb8b-7d81-443d-aab0-bb1cca1cf29e
file format: qcow2
virtual size: 80G (85899345920 bytes)
disk size: 1.2G
cluster_size: 65536
Format specific information:
compat: 1.1
lazy refcounts: false
refcount bits: 16
corrupt: false

2: After boot, the backingStore format is not expected 
..
/usr/libexec/qemu-kvm

  
  
  



  
  
  
      
  

..

>>
>> Signed-off-by: Yi Li 
>> ---
>>  src/qemu/qemu_block.c | 2 --
>>  src/util/virstoragefile.c | 4 +---
>>  src/util/virstoragefile.h | 1 -
>>  3 files changed, 1 insertion(+), 6 deletions(-)
>>
>> @@ -4916,8 +4916,6 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr 
>> src,
>>  goto cleanup;
>> 
>>  if (backingFormat == VIR_STORAGE_FILE_AUTO)
>> -backingStore->format = VIR_STORAGE_FILE_RAW;
>> -else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE)
>>  backingStore->format = VIR_STORAGE_FILE_AUTO;
> 
>I don't think we can do this safely. This code was added so that we
>never let qemu probe the image format. This was due to a security issue
>as a malicious guest could write a qcow2 or any other storage format
>header which has backing files into a raw volume. At new start this
>would be detected as the qcow2 or other format and qemu would open also
>the backing file. The guest then would gain access to un-allowed
>resources.
> 
>While now qemu refuses writing some parts of the raw image if no format
>was specified I don't think we should remove this code. Users always
>shall use the correct format.
> 
>NACK


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] util: storage: drop VIR_STORAGE_FILE_AUTO_SAFE

[Yi Li]

merge VIR_STORAGE_FILE_AUTO_SAFE/VIR_STORAGE_FILE_AUTO to VIR_STORAGE_FILE_AUTO
virStorageFileProbeFormatFromBuf will probe the backingStore format.

Fix the booting issue when setting backingStore format (QCOW image) to RAW 
image.

Signed-off-by: Yi Li 
---
 src/qemu/qemu_block.c | 2 --
 src/util/virstoragefile.c | 4 +---
 src/util/virstoragefile.h | 1 -
 3 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 66b1d11..0b99efc 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1367,7 +1367,6 @@ 
qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSourcePtr src)
 driver = virStorageFileFormatTypeToString(src->format);
 break;
 
-case VIR_STORAGE_FILE_AUTO_SAFE:
 case VIR_STORAGE_FILE_AUTO:
 case VIR_STORAGE_FILE_NONE:
 case VIR_STORAGE_FILE_COW:
@@ -2275,7 +2274,6 @@ 
qemuBlockStorageSourceCreateGetFormatProps(virStorageSourcePtr src,
 case VIR_STORAGE_FILE_DIR:
 return 0;
 
-case VIR_STORAGE_FILE_AUTO_SAFE:
 case VIR_STORAGE_FILE_AUTO:
 case VIR_STORAGE_FILE_NONE:
 virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 7288e18..2ebb2f1 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -692,7 +692,7 @@ qedGetBackingStore(char **res,
 if (flags & QED_F_BACKING_FORMAT_NO_PROBE)
 *format = VIR_STORAGE_FILE_RAW;
 else
-*format = VIR_STORAGE_FILE_AUTO_SAFE;
+*format = VIR_STORAGE_FILE_AUTO;
 
 return BACKING_STORE_OK;
 }
@@ -4916,8 +4916,6 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
 goto cleanup;
 
 if (backingFormat == VIR_STORAGE_FILE_AUTO)
-backingStore->format = VIR_STORAGE_FILE_RAW;
-else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE)
 backingStore->format = VIR_STORAGE_FILE_AUTO;
 else
 backingStore->format = backingFormat;
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index b65cd4c..c9deb6f 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -60,7 +60,6 @@ VIR_ENUM_DECL(virStorage);
 
 
 typedef enum {
-VIR_STORAGE_FILE_AUTO_SAFE = -2,
 VIR_STORAGE_FILE_AUTO = -1,
 VIR_STORAGE_FILE_NONE = 0,
 VIR_STORAGE_FILE_RAW,
-- 
2.7.5



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] storage: omit comma for ceph mon hosts to librados

[Yi Li]

Silly mistakes omit the comma for multiple ipaddr
Fixes: cdd362e0e7a34d4f8f102c75f2ca513d23dd1db0

Signed-off-by: Yi Li 
---
 src/storage/storage_backend_rbd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index bd6e3d5..315bef2 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -272,10 +272,10 @@ 
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
 const char *incFormat;
 if (virSocketAddrNumericFamily(source->hosts[i].name) == AF_INET6) 
{
 /* IPv6 address must be escaped in brackets on the cmd line */
-incFormat = "[%s]:%d";
+incFormat = "[%s]:%d,";
 } else {
 /* listenAddress is a hostname or IPv4 */
-incFormat = "%s:%d";
+incFormat = "%s:%d,";
 }
 virBufferAsprintf(_host, incFormat,
   source->hosts[i].name,
-- 
2.7.5



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] storage: escape ipv6 for ceph mon hosts to librados

[Yi Li]

>From 4af765cb6ee87eb7a131901057a8b6d0e859ac63 Mon Sep 17 00:00:00 2001
From: Yi Li 
Date: Sun, 28 Apr 2019 10:29:53 +0800
Subject: [PATCH v2] storage: escape ipv6 for ceph mon hosts to librados

Hosts for rbd are ceph monitor daemons. These have fixed IP addresses,
so they are often referenced by IP rather than hostname for
convenience, or to avoid relying on DNS. Using IPv4 addresses as the
host name works already, but IPv6 addresses require rbd-specific
escaping because the colon is used as an option separator in the
string passed to librados.

Escape these colons, and enclose the IPv6 address in square brackets
so it is distinguished from the port, which is currently mandatory.

Signed-off-by: Yi Li 
---
 docs/schemas/storagepool.rng  |  5 -
 src/storage/storage_backend_rbd.c | 15 ---
 tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml  | 13 +
 tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml | 16 
 tests/storagepoolxml2xmltest.c|  1 +
 5 files changed, 46 insertions(+), 4 deletions(-)
 create mode 100644 tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml
 create mode 100644 tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml

diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index 3ca8e79..976a02b 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -305,7 +305,10 @@
 
   
 
-  
+  
+
+
+  
 
 
   
diff --git a/src/storage/storage_backend_rbd.c
b/src/storage/storage_backend_rbd.c
index f8c968e..d6d7356 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -261,6 +261,7 @@
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
 VIR_DEBUG("Found %zu RADOS cluster monitors in the pool configuration",
   source->nhost);

+/* combine host and port into portal */
 for (i = 0; i < source->nhost; i++) {
 if (source->hosts[i].name != NULL &&
 !source->hosts[i].port) {
@@ -268,9 +269,17 @@
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
   source->hosts[i].name);
 } else if (source->hosts[i].name != NULL &&
 source->hosts[i].port) {
-virBufferAsprintf(_host, "%s:%d,",
-  source->hosts[i].name,
-  source->hosts[i].port);
+const char *incFormat;
+if (virSocketAddrNumericFamily(source->hosts[i].name) ==
AF_INET6) {
+/* IPv6 address must be escaped in brackets on the cmd line */
+incFormat = "[%s]:%d";
+} else {
+/* listenAddress is a hostname or IPv4 */
+incFormat = "%s:%d";
+}
+virBufferAsprintf(_host, incFormat,
+source->hosts[i].name,
+source->hosts[i].port);
 } else {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("received malformed monitor, check the
XML definition"));
diff --git a/tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml
b/tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml
new file mode 100644
index 000..0744b33
--- /dev/null
+++ b/tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml
@@ -0,0 +1,13 @@
+
+  ceph
+  47c1faee-0207-e741-f5ae-d9b019b98fe2
+  
+rbd
+
+
+
+
+  
+
+  
+
diff --git a/tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml
b/tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml
new file mode 100644
index 000..cc2a379
--- /dev/null
+++ b/tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml
@@ -0,0 +1,16 @@
+
+  ceph
+  47c1faee-0207-e741-f5ae-d9b019b98fe2
+  0
+  0
+  0
+  
+
+
+
+rbd
+
+  
+
+  
+
diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c
index 2ae514f..b6f4cb4 100644
--- a/tests/storagepoolxml2xmltest.c
+++ b/tests/storagepoolxml2xmltest.c
@@ -95,6 +95,7 @@ mymain(void)
 DO_TEST("pool-zfs-sourcedev");
 DO_TEST("pool-rbd");
 #ifdef WITH_STORAGE_RBD
+DO_TEST("pool-rbd-ipv6");
 DO_TEST("pool-rbd-refresh-volume-allocation");
 DO_TEST("pool-rbd-ns-configopts");
 #endif

--
2.7.5


On Sun, Apr 28, 2019 at 1:21 PM Yi Li  wrote:
>
> > >Hosts for rbd are ceph monitor daemons. These have fixed IP addresses,
> > >so they are often referenced by IP rather than hostname for
> > >convenience, or to avoid relying on DNS. Using IPv4 addresses as the
> > >host name works already, but IPv6 addresses require rbd-specific
> >
> > If you include the escaping in the XML, does it currently work?
> >   
>
> Yes, It works.
> >
> >

Re: [libvirt] [PATCH] storage: escape ipv6 for ceph mon hosts to librados

[Yi Li]

> >Hosts for rbd are ceph monitor daemons. These have fixed IP addresses,
> >so they are often referenced by IP rather than hostname for
> >convenience, or to avoid relying on DNS. Using IPv4 addresses as the
> >host name works already, but IPv6 addresses require rbd-specific
>
> If you include the escaping in the XML, does it currently work?
>   

Yes, It works.
>
>
> >escaping because the colon is used as an option separator in the
> >string passed to librados.
> >
> >Escape these colons, and enclose the IPv6 address in square brackets
> >so it is distinguished from the port, which is currently mandatory.
> >
> >Signed-off-by: Yi Li 
> >---
> > docs/schemas/storagepool.rng  |  5 -
> > src/storage/storage_backend_rbd.c | 13 ++---
> > tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml  | 13 +
> > tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml | 16 
> > tests/storagepoolxml2xmltest.c|  1 +
> > 5 files changed, 44 insertions(+), 4 deletions(-)
> > create mode 100644 tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml
> > create mode 100644 tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml
> >
> >diff --git a/src/storage/storage_backend_rbd.c 
> >b/src/storage/storage_backend_rbd.c
> >index f8c968e..3056563 100644
> >--- a/src/storage/storage_backend_rbd.c
> >+++ b/src/storage/storage_backend_rbd.c
> >@@ -268,9 +268,16 @@ 
> >virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
> >   source->hosts[i].name);
> > } else if (source->hosts[i].name != NULL &&
> > source->hosts[i].port) {
> >-virBufferAsprintf(_host, "%s:%d,",
> >-  source->hosts[i].name,
> >-  source->hosts[i].port);
> >+/* assume host containing : is ipv6 */
> >+if (strchr(source->hosts[i].name, ':')) {
>
> if (virSocketAddrNumericFamily(listenAddress) == AF_INET6)
>
> By using this helper function, we won't try to escape an address that is
> already escaped.
>
> Also, instead of copying the whole virBuffer call twice, it would be
> nicer to assign the format to a temporary variable like we do in 
> qemuMigrationDstPrepare
>
> Jano
Good point. I'd sending  a v2.
>
>
> >+virBufferAsprintf(_host, "[%s]:%d,",
> >+  source->hosts[i].name,
> >+  source->hosts[i].port);
> >+} else {
> >+virBufferAsprintf(_host, "%s:%d,",
> >+  source->hosts[i].name,
> >+  source->hosts[i].port);
> >+}
> > } else {
> > virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >_("received malformed monitor, check the XML 
> > definition"));


On Sun, Apr 28, 2019 at 1:10 PM winhong-yili  wrote:
>
> >Hosts for rbd are ceph monitor daemons. These have fixed IP addresses,
> >so they are often referenced by IP rather than hostname for
> >convenience, or to avoid relying on DNS. Using IPv4 addresses as the
> >host name works already, but IPv6 addresses require rbd-specific
>
> If you include the escaping in the XML, does it currently work?
>   
>
> >escaping because the colon is used as an option separator in the
> >string passed to librados.
> >
> >Escape these colons, and enclose the IPv6 address in square brackets
> >so it is distinguished from the port, which is currently mandatory.
> >
> >Signed-off-by: Yi Li 
> >---
> > docs/schemas/storagepool.rng  |  5 -
> > src/storage/storage_backend_rbd.c | 13 ++---
> > tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml  | 13 +
> > tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml | 16 
> > tests/storagepoolxml2xmltest.c|  1 +
> > 5 files changed, 44 insertions(+), 4 deletions(-)
> > create mode 100644 tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml
> > create mode 100644 tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml
> >
> >diff --git a/src/storage/storage_backend_rbd.c 
> >b/src/storage/storage_backend_rbd.c
> >index f8c968e..3056563 100644
> >--- a/src/storage/storage_backend_rbd.c
> >+++ b/src/storage/storage_backend_rbd.c
> >@@ -268,9 +268,16 @@ 
> >virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr pt

[libvirt] [PATCH] storage: escape ipv6 for ceph mon hosts to librados

[Yi Li]

Hosts for rbd are ceph monitor daemons. These have fixed IP addresses,
so they are often referenced by IP rather than hostname for
convenience, or to avoid relying on DNS. Using IPv4 addresses as the
host name works already, but IPv6 addresses require rbd-specific
escaping because the colon is used as an option separator in the
string passed to librados.

Escape these colons, and enclose the IPv6 address in square brackets
so it is distinguished from the port, which is currently mandatory.

Signed-off-by: Yi Li 
---
 docs/schemas/storagepool.rng  |  5 -
 src/storage/storage_backend_rbd.c | 13 ++---
 tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml  | 13 +
 tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml | 16 
 tests/storagepoolxml2xmltest.c|  1 +
 5 files changed, 44 insertions(+), 4 deletions(-)
 create mode 100644 tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml
 create mode 100644 tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml

diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index 3ca8e79..976a02b 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -305,7 +305,10 @@
 
   
 
-  
+  
+
+
+  
 
 
   
diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index f8c968e..3056563 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -268,9 +268,16 @@ 
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
   source->hosts[i].name);
 } else if (source->hosts[i].name != NULL &&
 source->hosts[i].port) {
-virBufferAsprintf(_host, "%s:%d,",
-  source->hosts[i].name,
-  source->hosts[i].port);
+/* assume host containing : is ipv6 */
+if (strchr(source->hosts[i].name, ':')) {
+virBufferAsprintf(_host, "[%s]:%d,",
+  source->hosts[i].name,
+  source->hosts[i].port);
+} else {
+virBufferAsprintf(_host, "%s:%d,",
+  source->hosts[i].name,
+  source->hosts[i].port);
+}
 } else {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("received malformed monitor, check the XML 
definition"));
diff --git a/tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml 
b/tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml
new file mode 100644
index 000..0744b33
--- /dev/null
+++ b/tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml
@@ -0,0 +1,13 @@
+
+  ceph
+  47c1faee-0207-e741-f5ae-d9b019b98fe2
+  
+rbd
+
+
+
+
+  
+
+  
+
diff --git a/tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml 
b/tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml
new file mode 100644
index 000..cc2a379
--- /dev/null
+++ b/tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml
@@ -0,0 +1,16 @@
+
+  ceph
+  47c1faee-0207-e741-f5ae-d9b019b98fe2
+  0
+  0
+  0
+  
+
+
+
+rbd
+
+  
+
+  
+
diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c
index 2ae514f..b6f4cb4 100644
--- a/tests/storagepoolxml2xmltest.c
+++ b/tests/storagepoolxml2xmltest.c
@@ -95,6 +95,7 @@ mymain(void)
 DO_TEST("pool-zfs-sourcedev");
 DO_TEST("pool-rbd");
 #ifdef WITH_STORAGE_RBD
+DO_TEST("pool-rbd-ipv6");
 DO_TEST("pool-rbd-refresh-volume-allocation");
 DO_TEST("pool-rbd-ns-configopts");
 #endif
-- 
2.7.5



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list