[libvirt] [PATCH 02/10] src: Use consistent error preservation and restoration calls

[John Ferlan]

Provide some consistency over error message variable name and usage
when saving error messages across possible other errors or possibility
of resetting of the last error.

Instead of virSaveLastError paired up with virSetError and virFreeError,
we should use the newer virErrorPreserveLast and virRestoreError.

Signed-off-by: John Ferlan 
---
 src/libvirt-stream.c | 36 
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/src/libvirt-stream.c b/src/libvirt-stream.c
index ef83696bcd..fb646d0aef 100644
--- a/src/libvirt-stream.c
+++ b/src/libvirt-stream.c
@@ -622,12 +622,11 @@ virStreamSendAll(virStreamPtr stream,
 VIR_FREE(bytes);
 
 if (ret != 0) {
-virErrorPtr orig_err = virSaveLastError();
+virErrorPtr orig_err;
+
+virErrorPreserveLast(&orig_err);
 virStreamAbort(stream);
-if (orig_err) {
-virSetError(orig_err);
-virFreeError(orig_err);
-}
+virErrorRestore(&orig_err);
 virDispatchError(stream->conn);
 }
 
@@ -794,12 +793,11 @@ int virStreamSparseSendAll(virStreamPtr stream,
 VIR_FREE(bytes);
 
 if (ret != 0) {
-virErrorPtr orig_err = virSaveLastError();
+virErrorPtr orig_err;
+
+virErrorPreserveLast(&orig_err);
 virStreamAbort(stream);
-if (orig_err) {
-virSetError(orig_err);
-virFreeError(orig_err);
-}
+virErrorRestore(&orig_err);
 virDispatchError(stream->conn);
 }
 
@@ -900,12 +898,11 @@ virStreamRecvAll(virStreamPtr stream,
 VIR_FREE(bytes);
 
 if (ret != 0) {
-virErrorPtr orig_err = virSaveLastError();
+virErrorPtr orig_err;
+
+virErrorPreserveLast(&orig_err);
 virStreamAbort(stream);
-if (orig_err) {
-virSetError(orig_err);
-virFreeError(orig_err);
-}
+virErrorRestore(&orig_err);
 virDispatchError(stream->conn);
 }
 
@@ -1034,12 +1031,11 @@ virStreamSparseRecvAll(virStreamPtr stream,
 VIR_FREE(bytes);
 
 if (ret != 0) {
-virErrorPtr orig_err = virSaveLastError();
+virErrorPtr orig_err;
+
+virErrorPreserveLast(&orig_err);
 virStreamAbort(stream);
-if (orig_err) {
-virSetError(orig_err);
-virFreeError(orig_err);
-}
+virErrorRestore(&orig_err);
 virDispatchError(stream->conn);
 }
 
-- 
2.20.1

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

[libvirt] [PATCH 01/10] conf: Use consistent error preservation and restoration calls

[John Ferlan]

Provide some consistency over error message variable name and usage
when saving error messages across possible other errors or possibility
of resetting of the last error.

Instead of virSaveLastError paired up with virSetError and virFreeError,
we should use the newer virErrorPreserveLast and virRestoreError.

Signed-off-by: John Ferlan 
---
 src/conf/domain_conf.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 10d6bf0eea..188ea80c3d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23624,17 +23624,14 @@ virDomainDefCheckABIStabilityFlags(virDomainDefPtr 
src,
 return true;
 
  error:
-err = virSaveLastError();
+virErrorPreserveLast(&err);
 
 strSrc = virDomainDefFormat(src, NULL, 0);
 strDst = virDomainDefFormat(dst, NULL, 0);
 VIR_DEBUG("XMLs that failed stability check were: src=\"%s\", dst=\"%s\"",
   NULLSTR(strSrc), NULLSTR(strDst));
 
-if (err) {
-virSetError(err);
-virFreeError(err);
-}
+virErrorRestore(&err);
 return false;
 }
 
-- 
2.20.1

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

[libvirt] [PATCH 03/10] libxl: Use consistent error preservation and restoration calls

[John Ferlan]

Provide some consistency over error message variable name and usage
when saving error messages across possible other errors or possibility
of resetting of the last error.

Instead of virSaveLastError paired up with virSetError and virFreeError,
we should use the newer virErrorPreserveLast and virRestoreError.

Signed-off-by: John Ferlan 
---
 src/libxl/libxl_migration.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 8a41e9374d..770a300316 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -1049,7 +1049,7 @@ libxlDoMigrateSrcP2P(libxlDriverPrivatePtr driver,
 if (uri_out) {
 if (virTypedParamsReplaceString(¶ms, &nparams,
 VIR_MIGRATE_PARAM_URI, uri_out) < 
0) {
-orig_err = virSaveLastError();
+virErrorPreserveLast(&orig_err);
 goto finish;
 }
 } else {
@@ -1067,7 +1067,7 @@ libxlDoMigrateSrcP2P(libxlDriverPrivatePtr driver,
  uri_out, NULL, flags);
 if (ret < 0) {
 notify_source = false;
-orig_err = virSaveLastError();
+virErrorPreserveLast(&orig_err);
 }
 
 cancelled = (ret < 0);
@@ -1094,7 +1094,7 @@ libxlDoMigrateSrcP2P(libxlDriverPrivatePtr driver,
  * one we need to preserve it in case confirm3 overwrites
  */
 if (!orig_err)
-orig_err = virSaveLastError();
+virErrorPreserveLast(&orig_err);
 
  confirm:
 if (notify_source) {
@@ -1119,10 +1119,7 @@ libxlDoMigrateSrcP2P(libxlDriverPrivatePtr driver,
 ret = -1;
 }
 
-if (orig_err) {
-virSetError(orig_err);
-virFreeError(orig_err);
-}
+virErrorRestore(&orig_err);
 
 VIR_FREE(cookieout);
 VIR_FREE(dom_xml);
@@ -1200,15 +1197,12 @@ libxlDomainMigrationSrcPerformP2P(libxlDriverPrivatePtr 
driver,
 }
 
  cleanup:
-orig_err = virSaveLastError();
+virErrorPreserveLast(&orig_err);
 virObjectUnlock(vm);
 virObjectUnref(dconn);
 virObjectUnref(cfg);
 virObjectLock(vm);
-if (orig_err) {
-virSetError(orig_err);
-virFreeError(orig_err);
-}
+virErrorRestore(&orig_err);
 return ret;
 }
 
-- 
2.20.1

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

[libvirt] [PATCH 04/10] lxc: Use consistent error preservation and restoration calls

[John Ferlan]

Provide some consistency over error message variable name and usage
when saving error messages across possible other errors or possibility
of resetting of the last error.

Instead of virSaveLastError paired up with virSetError and virFreeError,
we should use the newer virErrorPreserveLast and virRestoreError.

Signed-off-by: John Ferlan 
---
 src/lxc/lxc_driver.c  | 9 -
 src/lxc/lxc_process.c | 7 ++-
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 5c7a9140b2..204a3ed522 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1857,12 +1857,11 @@ static int lxcSetVcpuBWLive(virCgroupPtr cgroup, 
unsigned long long period,
 
  error:
 if (period) {
-virErrorPtr saved = virSaveLastError();
+virErrorPtr saved;
+
+virErrorPreserveLast(&saved);
 virCgroupSetCpuCfsPeriod(cgroup, old_period);
-if (saved) {
-virSetError(saved);
-virFreeError(saved);
-}
+virErrorRestore(&saved);
 }
 
 return -1;
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index da0b055b85..d9939f102d 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1568,7 +1568,7 @@ int virLXCProcessStart(virConnectPtr conn,
 rc = -1;
 }
 if (rc != 0) {
-err = virSaveLastError();
+virErrorPreserveLast(&err);
 virLXCProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED);
 }
 virCommandFree(cmd);
@@ -1582,10 +1582,7 @@ int virLXCProcessStart(virConnectPtr conn,
 virObjectUnref(cfg);
 virObjectUnref(caps);
 
-if (err) {
-virSetError(err);
-virFreeError(err);
-}
+virErrorRestore(&err);
 
 return rc;
 }
-- 
2.20.1

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

[libvirt] [PATCH 06/10] remote: Use consistent error preservation and restoration calls

[John Ferlan]

Provide some consistency over error message variable name and usage
when saving error messages across possible other errors or possibility
of resetting of the last error.

Instead of virSaveLastError paired up with virSetError and virFreeError,
we should use the newer virErrorPreserveLast and virRestoreError.

Signed-off-by: John Ferlan 
---
 src/remote/remote_daemon_stream.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/remote/remote_daemon_stream.c 
b/src/remote/remote_daemon_stream.c
index d7fcb1bf42..c4f14a27ef 100644
--- a/src/remote/remote_daemon_stream.c
+++ b/src/remote/remote_daemon_stream.c
@@ -229,15 +229,18 @@ daemonStreamEvent(virStreamPtr st, int events, void 
*opaque)
 int ret;
 virNetMessagePtr msg;
 virNetMessageError rerr;
-virErrorPtr origErr = virSaveLastError();
+virErrorPtr origErr;
+
+virErrorPreserveLast(&origErr);
 
 memset(&rerr, 0, sizeof(rerr));
 stream->closed = true;
 virStreamEventRemoveCallback(stream->st);
 virStreamAbort(stream->st);
 if (origErr && origErr->code != VIR_ERR_OK) {
-virSetError(origErr);
+virErrorRestore(&origErr);
 } else {
+virFreeError(origErr);
 if (events & VIR_STREAM_EVENT_HANGUP)
 virReportError(VIR_ERR_RPC,
"%s", _("stream had unexpected termination"));
@@ -245,7 +248,6 @@ daemonStreamEvent(virStreamPtr st, int events, void *opaque)
 virReportError(VIR_ERR_RPC,
"%s", _("stream had I/O failure"));
 }
-virFreeError(origErr);
 
 msg = virNetMessageNew(false);
 if (!msg) {
@@ -549,7 +551,9 @@ daemonStreamHandleWriteData(virNetServerClientPtr client,
 return 1;
 } else if (ret < 0) {
 virNetMessageError rerr;
-virErrorPtr err = virSaveLastError();
+virErrorPtr err;
+
+virErrorPreserveLast(&err);
 
 memset(&rerr, 0, sizeof(rerr));
 
@@ -558,10 +562,7 @@ daemonStreamHandleWriteData(virNetServerClientPtr client,
 virStreamEventRemoveCallback(stream->st);
 virStreamAbort(stream->st);
 
-if (err) {
-virSetError(err);
-virFreeError(err);
-}
+virErrorRestore(&err);
 
 return virNetServerProgramSendReplyError(stream->prog,
  client,
-- 
2.20.1

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

[libvirt] [PATCH 08/10] util: Use consistent error preservation and restoration calls

[John Ferlan]

Provide some consistency over error message variable name and usage
when saving error messages across possible other errors or possibility
of resetting of the last error.

Instead of virSaveLastError paired up with virSetError and virFreeError,
we should use the newer virErrorPreserveLast and virRestoreError.

Signed-off-by: John Ferlan 
---
 src/util/vircgroup.c   | 18 --
 src/util/virfirewall.c |  6 --
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index c6e4cf2dde..4c83e37ec4 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1168,13 +1168,12 @@ virCgroupNewMachineSystemd(const char *name,
 }
 
 if (virCgroupAddProcess(*group, pidleader) < 0) {
-virErrorPtr saved = virSaveLastError();
+virErrorPtr saved;
+
+virErrorPreserveLast(&saved);
 virCgroupRemove(*group);
 virCgroupFree(group);
-if (saved) {
-virSetError(saved);
-virFreeError(saved);
-}
+virErrorRestore(&saved);
 }
 
 return 0;
@@ -1220,13 +1219,12 @@ virCgroupNewMachineManual(const char *name,
 goto cleanup;
 
 if (virCgroupAddProcess(*group, pidleader) < 0) {
-virErrorPtr saved = virSaveLastError();
+virErrorPtr saved;
+
+virErrorPreserveLast(&saved);
 virCgroupRemove(*group);
 virCgroupFree(group);
-if (saved) {
-virSetError(saved);
-virFreeError(saved);
-}
+virErrorRestore(&saved);
 }
 
  done:
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 0d4bfae8f8..62f404afd6 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -838,7 +838,9 @@ virFirewallApply(virFirewallPtr firewall)
 if (virFirewallApplyGroup(firewall, i) < 0) {
 VIR_DEBUG("Rolling back groups up to %zu for %p", i, firewall);
 size_t first = i;
-VIR_AUTOPTR(virError) saved_error = virSaveLastError();
+virErrorPtr saved_error;
+
+virErrorPreserveLast(&saved_error);
 
 /*
  * Look at any inheritance markers to figure out
@@ -858,7 +860,7 @@ virFirewallApply(virFirewallPtr firewall)
 virFirewallRollbackGroup(firewall, j);
 }
 
-virSetError(saved_error);
+virErrorRestore(&saved_error);
 VIR_DEBUG("Done rolling back groups for %p", firewall);
 goto cleanup;
 }
-- 
2.20.1

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

[libvirt] [PATCH 07/10] storage: Use consistent error preservation and restoration calls

[John Ferlan]

Provide some consistency over error message variable name and usage
when saving error messages across possible other errors or possibility
of resetting of the last error.

Instead of virSaveLastError paired up with virSetError and virFreeError,
we should use the newer virErrorPreserveLast and virRestoreError.

Signed-off-by: John Ferlan 
---
 src/storage/storage_backend_disk.c| 5 ++---
 src/storage/storage_backend_logical.c | 5 ++---
 src/storage/storage_driver.c  | 8 +++-
 3 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/src/storage/storage_backend_disk.c 
b/src/storage/storage_backend_disk.c
index 5bf704bcc8..d2f7b94224 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -918,10 +918,9 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool,
 /* Best effort to remove the partition. Ignore any errors
  * since we could be calling this with vol->target.path == NULL
  */
-save_err = virSaveLastError();
+virErrorPreserveLast(&save_err);
 ignore_value(virStorageBackendDiskDeleteVol(pool, vol, 0));
-virSetError(save_err);
-virFreeError(save_err);
+virErrorRestore(&save_err);
 return -1;
 }
 
diff --git a/src/storage/storage_backend_logical.c 
b/src/storage/storage_backend_logical.c
index 6e468b3579..bfedb06a91 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -969,10 +969,9 @@ virStorageBackendLogicalCreateVol(virStoragePoolObjPtr 
pool,
 return 0;
 
  error:
-err = virSaveLastError();
+virErrorPreserveLast(&err);
 virStorageBackendLogicalDeleteVol(pool, vol, 0);
-virSetError(err);
-virFreeError(err);
+virErrorRestore(&err);
 return -1;
 }
 
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 6516b0943d..e8551ba57e 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -81,18 +81,16 @@ storagePoolRefreshFailCleanup(virStorageBackendPtr backend,
   virStoragePoolObjPtr obj,
   const char *stateFile)
 {
-virErrorPtr orig_err = virSaveLastError();
+virErrorPtr orig_err;
 
+virErrorPreserveLast(&orig_err);
 virStoragePoolObjClearVols(obj);
 
 if (stateFile)
 unlink(stateFile);
 if (backend->stopPool)
 backend->stopPool(obj);
-if (orig_err) {
-virSetError(orig_err);
-virFreeError(orig_err);
-}
+virErrorRestore(&orig_err);
 }
 
 
-- 
2.20.1

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

[libvirt] [PATCH 09/10] vz: Use consistent error preservation and restoration calls

[John Ferlan]

Provide some consistency over error message variable name and usage
when saving error messages across possible other errors or possibility
of resetting of the last error.

Instead of virSaveLastError paired up with virSetError and virFreeError,
we should use the newer virErrorPreserveLast and virRestoreError.

Signed-off-by: John Ferlan 
---
 src/vz/vz_driver.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 917acdae57..b4049261ae 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -3206,7 +3206,7 @@ vzDomainMigratePerformP2P(virDomainObjPtr dom,
  */
 if (uri && virTypedParamsReplaceString(¶ms, &nparams,
VIR_MIGRATE_PARAM_URI, uri) < 0) {
-orig_err = virSaveLastError();
+virErrorPreserveLast(&orig_err);
 goto finish;
 }
 
@@ -3216,7 +3216,7 @@ vzDomainMigratePerformP2P(virDomainObjPtr dom,
 cookieoutlen = 0;
 if (vzDomainMigratePerformStep(dom, driver, params, nparams, cookiein,
cookieinlen, flags) < 0) {
-orig_err = virSaveLastError();
+virErrorPreserveLast(&orig_err);
 goto finish;
 }
 
@@ -3242,10 +3242,7 @@ vzDomainMigratePerformP2P(virDomainObjPtr dom,
 /* confirm step is NOOP thus no need to call it */
 
  done:
-if (orig_err) {
-virSetError(orig_err);
-virFreeError(orig_err);
-}
+virErrorRestore(&orig_err);
 VIR_FREE(dom_xml);
 VIR_FREE(uri);
 VIR_FREE(cookiein);
-- 
2.20.1

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

[libvirt] [PATCH 00/10] Clean up error preservation/restoration

[John Ferlan]

I was going through some old git trees and found this one started before
I 'altered' my Red Hat career path. Seeing as no one has picked it up from
https://wiki.libvirt.org/page/BiteSizedTasks - figured I may as well post
it rather than throw it away ;-)

The change essentially alters virSaveLastError ... virSetError and
virFreeError paths to use virErrorPreserveLast & virErrorRestore.
 

John Ferlan (10):
  conf: Use consistent error preservation and restoration calls
  src: Use consistent error preservation and restoration calls
  libxl: Use consistent error preservation and restoration calls
  lxc: Use consistent error preservation and restoration calls
  qemu: Use consistent error preservation and restoration calls
  remote: Use consistent error preservation and restoration calls
  storage: Use consistent error preservation and restoration calls
  util: Use consistent error preservation and restoration calls
  vz: Use consistent error preservation and restoration calls
  tools: Use consistent error preservation and restoration calls

 src/conf/domain_conf.c|  7 +--
 src/libvirt-stream.c  | 36 +---
 src/libxl/libxl_migration.c   | 18 ++
 src/lxc/lxc_driver.c  |  9 ++-
 src/lxc/lxc_process.c |  7 +--
 src/qemu/qemu_cgroup.c| 14 ++---
 src/qemu/qemu_command.c   | 12 ++--
 src/qemu/qemu_domain.c|  7 +--
 src/qemu/qemu_driver.c| 36 +---
 src/qemu/qemu_migration.c | 85 +--
 src/qemu/qemu_migration_params.c  |  9 ++-
 src/qemu/qemu_monitor.c   | 19 +++---
 src/qemu/qemu_process.c   |  7 +--
 src/remote/remote_daemon_stream.c | 17 +++---
 src/storage/storage_backend_disk.c|  5 +-
 src/storage/storage_backend_logical.c |  5 +-
 src/storage/storage_driver.c  |  8 +--
 src/util/vircgroup.c  | 18 +++---
 src/util/virfirewall.c|  6 +-
 src/vz/vz_driver.c|  9 +--
 tools/virsh.c |  7 +--
 tools/virt-admin.c|  7 +--
 22 files changed, 137 insertions(+), 211 deletions(-)

-- 
2.20.1

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

[libvirt] [PATCH 10/10] tools: Use consistent error preservation and restoration calls

[John Ferlan]

Provide some consistency over error message variable name and usage
when saving error messages across possible other errors or possibility
of resetting of the last error.

Instead of virSaveLastError paired up with virSetError and virFreeError,
we should use the newer virErrorPreserveLast and virRestoreError.

Signed-off-by: John Ferlan 
---
 tools/virsh.c  | 7 ++-
 tools/virt-admin.c | 7 ++-
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 4aae877160..a3553ddd36 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -93,7 +93,7 @@ virshCatchDisconnect(virConnectPtr conn,
 virErrorPtr error;
 char *uri;
 
-error = virSaveLastError();
+virErrorPreserveLast(&error);
 uri = virConnectGetURI(conn);
 
 switch ((virConnectCloseReason) reason) {
@@ -114,10 +114,7 @@ virshCatchDisconnect(virConnectPtr conn,
 vshError(ctl, _(str), NULLSTR(uri));
 VIR_FREE(uri);
 
-if (error) {
-virSetError(error);
-virFreeError(error);
-}
+virErrorRestore(&error);
 disconnected++;
 vshEventDone(ctl);
 }
diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index c0cc0999cb..a820e7241d 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -124,7 +124,7 @@ vshAdmCatchDisconnect(virAdmConnectPtr conn G_GNUC_UNUSED,
 if (reason == VIR_CONNECT_CLOSE_REASON_CLIENT)
 return;
 
-error = virSaveLastError();
+virErrorPreserveLast(&error);
 uri = virAdmConnectGetURI(conn);
 
 switch ((virConnectCloseReason) reason) {
@@ -146,10 +146,7 @@ vshAdmCatchDisconnect(virAdmConnectPtr conn G_GNUC_UNUSED,
 vshError(ctl, _(str), NULLSTR(uri));
 VIR_FREE(uri);
 
-if (error) {
-virSetError(error);
-virFreeError(error);
-}
+virErrorRestore(&error);
 }
 
 static int
-- 
2.20.1

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

[libvirt] [PATCH 05/10] qemu: Use consistent error preservation and restoration calls

[John Ferlan]

Provide some consistency over error message variable name and usage
when saving error messages across possible other errors or possibility
of resetting of the last error.

Instead of virSaveLastError paired up with virSetError and virFreeError,
we should use the newer virErrorPreserveLast and virRestoreError.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_cgroup.c   | 14 +++---
 src/qemu/qemu_command.c  | 12 ++---
 src/qemu/qemu_domain.c   |  7 +--
 src/qemu/qemu_driver.c   | 36 ++
 src/qemu/qemu_migration.c| 85 +++-
 src/qemu/qemu_migration_params.c |  9 ++--
 src/qemu/qemu_monitor.c  | 19 +++
 src/qemu/qemu_process.c  |  7 +--
 8 files changed, 72 insertions(+), 117 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index f1776a7c0b..8b915d124c 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -1149,12 +1149,11 @@ qemuSetupCgroupVcpuBW(virCgroupPtr cgroup,
 
  error:
 if (period) {
-virErrorPtr saved = virSaveLastError();
+virErrorPtr saved;
+
+virErrorPreserveLast(&saved);
 ignore_value(virCgroupSetCpuCfsPeriod(cgroup, old_period));
-if (saved) {
-virSetError(saved);
-virFreeError(saved);
-}
+virErrorRestore(&saved);
 }
 
 return -1;
@@ -1362,10 +1361,9 @@ 
qemuCgroupEmulatorAllNodesRestore(qemuCgroupEmulatorAllNodesDataPtr data)
 if (!data)
 return;
 
-err = virSaveLastError();
+virErrorPreserveLast(&err);
 virCgroupSetCpusetMems(data->emulatorCgroup, data->emulatorMemMask);
-virSetError(err);
-virFreeError(err);
+virErrorRestore(&err);
 
 qemuCgroupEmulatorAllNodesDataFree(data);
 }
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c2a19689c5..2e5e2a75bf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8644,10 +8644,11 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
 ret = 0;
  cleanup:
 if (ret < 0) {
-virErrorPtr saved_err = virSaveLastError();
+virErrorPtr saved_err;
+
+virErrorPreserveLast(&saved_err);
 virDomainConfNWFilterTeardown(net);
-virSetError(saved_err);
-virFreeError(saved_err);
+virErrorRestore(&saved_err);
 }
 for (i = 0; vhostfd && i < vhostfdSize; i++) {
 if (ret < 0)
@@ -8730,11 +8731,10 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver,
  error:
 /* free up any resources in the network driver
  * but don't overwrite the original error */
-originalError = virSaveLastError();
+virErrorPreserveLast(&originalError);
 for (i = 0; last_good_net != -1 && i <= last_good_net; i++)
 virDomainConfNWFilterTeardown(def->nets[i]);
-virSetError(originalError);
-virFreeError(originalError);
+virErrorRestore(&originalError);
 return -1;
 }
 
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c638077aa8..43aec2e3d7 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9174,7 +9174,7 @@ void qemuDomainObjTaint(virQEMUDriverPtr driver,
 /* We don't care about errors logging taint info, so
  * preserve original error, and clear any error that
  * is raised */
-orig_err = virSaveLastError();
+virErrorPreserveLast(&orig_err);
 
 if (!(timestamp = virTimeStringNow()))
 goto cleanup;
@@ -9198,10 +9198,7 @@ void qemuDomainObjTaint(virQEMUDriverPtr driver,
 
  cleanup:
 VIR_FREE(timestamp);
-if (orig_err) {
-virSetError(orig_err);
-virFreeError(orig_err);
-}
+virErrorRestore(&orig_err);
 }
 
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6ce6348593..c662676be3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3436,7 +3436,8 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver,
  endjob:
 if (ret < 0) {
 if (was_running && virDomainObjIsActive(vm)) {
-virErrorPtr save_err = virSaveLastError();
+virErrorPtr save_err;
+virErrorPreserveLast(&save_err);
 if (qemuProcessStartCPUs(driver, vm,
  VIR_DOMAIN_RUNNING_SAVE_CANCELED,
  QEMU_ASYNC_JOB_SAVE) < 0) {
@@ -3446,8 +3447,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver,
  VIR_DOMAIN_EVENT_SUSPENDED,
  
VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR));
 }
-virSetError(save_err);
-virFreeError(save_err);
+virErrorRestore(&save_err);
 }
 }
 qemuDomainObjEndAsyncJob(driver, vm);
@@ -6729,7 +6729,9 @@ qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver,
 goto cleanup;
 
 if (!virDomainDefCheckABIStability(def, newdef_migr, driver->xmlopt)) {
-virErrorPtr err = virSaveLastError();
+virErrorPtr save_err;

Re: [libvirt] [RFC 0/1] convert virStorageSource to GObject

[Daniel Henrique Barboza]

On 10/15/19 2:15 PM, Daniel P. Berrangé wrote:

On Tue, Oct 15, 2019 at 10:51:39AM -0300, Daniel Henrique Barboza wrote:


On 10/15/19 9:55 AM, Daniel P. Berrangé wrote:

On Tue, Oct 15, 2019 at 09:42:45AM -0300, Daniel Henrique Barboza wrote:

I was hoping to quickly re-send the qemu_driver cleanups I've
sent some time ago, now using Glib. I started by attempting to
change the first VIR_AUTOUNREF() call in qemu_driver.c to
g_autoptr(), which happens to be a virStorageSourcePtr type,
then I realized that it wasn't that simple.

It should be that simple with this commit:

commit 667ff797e8eb8d82f30ab430216a8d2eef6b915a
Author: Daniel P. Berrangé 
Date:   Fri Oct 4 17:14:10 2019 +0100

  src: add support for g_autoptr with virObject instances

we should be able to use g_autoptr for any virObject, without
having to lock-step convert to GObject.

What actual problem did you find ?

I failed to notice this commit. Just tried it again and it worked.

What happened yesterday was that I attempted to do a simple
VIR_AUTOUNREF -> g_autopt replace, faced compile errors and then, since
I didn't notice this commit about, I assumed "I guess I need to convert
this guy to GObject".

In fact, the compile error happened because g_autoptr() does not operate
with a 'Ptr' type - something that I learned only during the conversion
process.

Yeah, you need to drop the 'Ptr' suffix in the type name when
converting to g_autoptr, as it adds the pointer itself.



This is being sent as RFC because x-I am aware that docs/hacking.html
mentions that we shouldn't mix up certain GLib macros with Libvirt
ones, thus I am uncertain of whether I have messed up or not.
'make check' works, did a few sanity checks with libvirtd as
well.

Yes, the need to not mix  g_auto* with VIR_AUTO*, is why I did commit
667ff797e8eb8d82f30ab430216a8d2eef6b915a to let you use g_autoptr
with virObject, without first converting to GObject.

What if there are other object types in the same function  using the VIR
macros?
For example, inside qemu_driver.c: qemuDomainBlockCopyCommon:


     VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg =
virQEMUDriverGetConfig(driver);
     const char *format = NULL;
     bool mirror_reuse = !!(flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT);
     bool mirror_shallow = !!(flags & VIR_DOMAIN_BLOCK_COPY_SHALLOW);
     bool existing = mirror_reuse;
     qemuBlockJobDataPtr job = NULL;
     VIR_AUTOUNREF(virStorageSourcePtr) mirror = mirrorsrc;
     bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
     bool mirror_initialized = false;
     VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL;
     VIR_AUTOPTR(qemuBlockStorageSourceChainData) crdata = NULL;


Let's say that I change the virStorageSourcePtr up there to

    g_autoptr(virStorageSource) mirror = mirrorsrc;


As long as there are no VIR macros acting in the 'mirror' variable, is it to
use g_autoptr
there even when everyone else is using VIR_AUTO* macros?

You should change all variables in the method at the same time.
Both the VIR_AUTOUNEF calls here can use g_autoptr, as can the
two VIR_AUTOPTR calls.


Thanks for clarifying. I was going to re-send the patches adding GLib
macros instead of VIR_AUTO* ones, which would end up breaking this
rule because these patches are changing stuff in smaller steps.

What I'll end up is to basically re-send them as they are now, but with an
extra patch to change everything to GLib at once. That way we'll stay
compliant every step of the way.




DHB




Regards,
Daniel


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

[libvirt] [PATCH v3 00/10] use VIR_AUTO*/g_auto* all around qemu_driver.c

[Daniel Henrique Barboza]

changes from v2:
- rebased with newer master (67e72053c1)
- added an extra patch to convert the existing VIR_AUTO* macros
to g_auto* ones, all at once, to avoid the case where a method
will have both VIR_AUTO* and g_auto* macros at the same time.


Note: the conversion in patch 10 wasn't 100% due to a handful of
methods that I was unable to use g_autoptr. Take for example
the method qemuDomainSaveInternal:
--
qemuDomainObjPrivatePtr priv = vm->privateData;
VIR_AUTOUNREF(virCapsPtr) caps = NULL;
virQEMUSaveDataPtr data = NULL;
VIR_AUTOUNREF(qemuDomainSaveCookiePtr) cookie = NULL;
--

Changing the 'cookie' variable to use g_autoptr() causes an error:

make[3]: Entering directory '/home/danielhb/kvm-project/libvirt/src'
  CC   qemu/libvirt_driver_qemu_impl_la-qemu_driver.lo
In file included from /usr/lib64/glib-2.0/include/glibconfig.h:9,
 from /usr/include/glib-2.0/glib/gtypes.h:32,
 from /usr/include/glib-2.0/glib/galloca.h:32,
 from /usr/include/glib-2.0/glib.h:30,
 from ./internal.h:31,
 from qemu/qemu_agent.h:24,
 from qemu/qemu_driver.c:40:
qemu/qemu_driver.c: In function 'qemuDomainSaveInternal':
qemu/qemu_driver.c:3282:15: error: unknown type name 
'qemuDomainSaveCookie_autoptr'
 3282 | g_autoptr(qemuDomainSaveCookie) cookie = NULL;


I tried doing it with the 'Ptr' in the name, same error:


  CC   qemu/libvirt_driver_qemu_impl_la-qemu_driver.lo
In file included from /usr/lib64/glib-2.0/include/glibconfig.h:9,
 from /usr/include/glib-2.0/glib/gtypes.h:32,
 from /usr/include/glib-2.0/glib/galloca.h:32,
 from /usr/include/glib-2.0/glib.h:30,
 from ./internal.h:31,
 from qemu/qemu_agent.h:24,
 from qemu/qemu_driver.c:40:
qemu/qemu_driver.c: In function 'qemuDomainSaveInternal':
qemu/qemu_driver.c:3282:15: error: unknown type name 
'qemuDomainSaveCookiePtr_autoptr'
 3282 | g_autoptr(qemuDomainSaveCookiePtr) cookie = NULL;


Similar situation happens with qemuDomainSaveImageStartVM and with
qemuSecurityInit methods. They are mentioned in the commit message
of patch 10 for reference. These methods are still using VIR_AUTO*
macros.


changes from v1:
- addressed review concerns made by Erik
- added more cleanups, as suggested by Erik

v2: https://www.redhat.com/archives/libvir-list/2019-September/msg01452.html
v1: https://www.redhat.com/archives/libvir-list/2019-September/msg00719.html

Daniel Henrique Barboza (10):
  qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 1/3
  qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 2/3
  qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 3/3
  qemu_driver: use VIR_AUTOUNREF() with virCapsPtr
  qemu_driver: use VIR_AUTOUNREF() with more pointer types
  qemu_driver: use VIR_AUTOFREE() with strings 1/3
  qemu_driver: use VIR_AUTOFREE() with strings 2/3
  qemu_driver: use VIR_AUTOFREE() with strings 3/3
  qemu_driver: VIR_AUTOFREE on other scalar pointers
  qemu_driver.c: use GLib macros

 src/qemu/qemu_driver.c | 755 ++---
 1 file changed, 259 insertions(+), 496 deletions(-)

-- 
2.21.0

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

[libvirt] [PATCH v3 09/10] qemu_driver: VIR_AUTOFREE on other scalar pointers

[Daniel Henrique Barboza]

This patch uses VIR_AUTOFREE on instances of 'unsigned int *'
and 'unsigned long long *' found in qemu_driver.c.

Suggested-by: Erik Skultety 
Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_driver.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fd7c8bb1be..6f283d2de3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1082,7 +1082,7 @@ qemuStateStop(void)
 size_t i;
 int state;
 virDomainPtr *domains = NULL;
-unsigned int *flags = NULL;
+VIR_AUTOFREE(unsigned int *) flags = NULL;
 VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = 
virQEMUDriverGetConfig(qemu_driver);
 
 if (!(conn = virConnectOpen(cfg->uri)))
@@ -1120,7 +1120,6 @@ qemuStateStop(void)
 virObjectUnref(domains[i]);
 VIR_FREE(domains);
 }
-VIR_FREE(flags);
 
 return ret;
 }
@@ -20715,7 +20714,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
 size_t i;
 int ret = -1;
 virVcpuInfoPtr cpuinfo = NULL;
-unsigned long long *cpuwait = NULL;
+VIR_AUTOFREE(unsigned long long *) cpuwait = NULL;
 
 if (virTypedParamListAddUInt(params, virDomainDefGetVcpus(dom->def),
  "vcpu.current") < 0)
@@ -20780,7 +20779,6 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
 
  cleanup:
 VIR_FREE(cpuinfo);
-VIR_FREE(cpuwait);
 return ret;
 }
 
-- 
2.21.0

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

[libvirt] [PATCH v3 02/10] qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 2/3

[Daniel Henrique Barboza]

virQEMUDriverConfigPtr can be auto-unref for the great majority
of the uses made in qemu_driver, sparing us a virObjectUnref()
call and sometimes a whole 'cleanup' label.

This patch changes virQEMUDriverConfigPtr declarations to
use VIR_AUTOUNREF(). 'cleanup' labels were deleted when
applicable.

Since there are a lot of references to change, let's do it in
3 steps. This is step 2.

Note: there is a g_autofree GLib macro being used in
qemuDomainUndefineFlags. In this particular instance we're
using g_autoptr() instead of VIR_AUTOUNREF() to not mix-up VIR_*
and GLib macros in that method.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_driver.c | 81 ++
 1 file changed, 26 insertions(+), 55 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1bd9609761..9f9443f493 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4571,7 +4571,7 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
virDomainObjPtr vm,
const char *devAlias)
 {
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virDomainDeviceDef dev;
 virDomainNetDefPtr def;
@@ -4670,7 +4670,6 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
  cleanup:
 virNetDevRxFilterFree(hostFilter);
 virNetDevRxFilterFree(guestFilter);
-virObjectUnref(cfg);
 }
 
 
@@ -4680,7 +4679,7 @@ processSerialChangedEvent(virQEMUDriverPtr driver,
   const char *devAlias,
   bool connected)
 {
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 virDomainChrDeviceState newstate;
 virObjectEventPtr event = NULL;
 virDomainDeviceDef dev;
@@ -4713,7 +4712,7 @@ processSerialChangedEvent(virQEMUDriverPtr driver,
 }
 
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
-goto cleanup;
+return;
 
 if (!virDomainObjIsActive(vm)) {
 VIR_DEBUG("Domain is not running");
@@ -4754,9 +4753,6 @@ processSerialChangedEvent(virQEMUDriverPtr driver,
 
  endjob:
 qemuDomainObjEndJob(driver, vm);
-
- cleanup:
-virObjectUnref(cfg);
 }
 
 
@@ -5029,21 +5025,20 @@ qemuDomainSetVcpusMax(virQEMUDriverPtr driver,
   virDomainDefPtr persistentDef,
   unsigned int nvcpus)
 {
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 unsigned int topologycpus;
-int ret = -1;
 
 if (def) {
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("maximum vcpu count of a live domain can't be 
modified"));
-goto cleanup;
+return -1;
 }
 
 if (virDomainNumaGetCPUCountTotal(persistentDef->numa) > nvcpus) {
 virReportError(VIR_ERR_INVALID_ARG, "%s",
_("Number of CPUs in  exceeds the desired "
  "maximum vcpu count"));
-goto cleanup;
+return -1;
 }
 
 if (virDomainDefGetVcpusTopology(persistentDef, &topologycpus) == 0 &&
@@ -5052,23 +5047,19 @@ qemuDomainSetVcpusMax(virQEMUDriverPtr driver,
  * setting may be corrected via this API */
 virReportError(VIR_ERR_INVALID_ARG, "%s",
_("CPU topology doesn't match the desired vcpu count"));
-goto cleanup;
+return -1;
 }
 
 /* ordering information may become invalid, thus clear it */
 virDomainDefVcpuOrderClear(persistentDef);
 
 if (virDomainDefSetVcpusMax(persistentDef, nvcpus, driver->xmlopt) < 0)
-goto cleanup;
+return -1;
 
 if (virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef) < 0)
-goto cleanup;
-
-ret = 0;
+return -1;
 
- cleanup:
-virObjectUnref(cfg);
-return ret;
+return 0;
 }
 
 
@@ -5233,7 +5224,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
 int ret = -1;
 virBitmapPtr pcpumap = NULL;
 virDomainVcpuDefPtr vcpuinfo = NULL;
-virQEMUDriverConfigPtr cfg = NULL;
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
 
 virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
   VIR_DOMAIN_AFFECT_CONFIG, -1);
@@ -5290,7 +5281,6 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
  cleanup:
 virDomainObjEndAPI(&vm);
 virBitmapFree(pcpumap);
-virObjectUnref(cfg);
 return ret;
 }
 
@@ -5353,7 +5343,7 @@ qemuDomainPinEmulator(virDomainPtr dom,
 int ret = -1;
 qemuDomainObjPrivatePtr priv;
 virBitmapPtr pcpumap = NULL;
-virQEMUDriverConfigPtr cfg = NULL;
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
 virObjectEventPtr event = NULL;
 char *str = NULL;
 virTypedPara

[libvirt] [PATCH v3 01/10] qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 1/3

[Daniel Henrique Barboza]

virQEMUDriverConfigPtr can be auto-unref for the great majority
of the uses made in qemu_driver, sparing us a virObjectUnref()
call and sometimes a whole 'cleanup' label.

This patch changes virQEMUDriverConfigPtr declarations to
use VIR_AUTOUNREF(). 'cleanup' labels were deleted when
applicable. Since there are a lot of references to change,
let's do it in 3 steps.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_driver.c | 94 +-
 1 file changed, 29 insertions(+), 65 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6ce6348593..1bd9609761 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -202,7 +202,7 @@ qemuAutostartDomain(virDomainObjPtr vm,
 {
 virQEMUDriverPtr driver = opaque;
 int flags = 0;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 int ret = -1;
 
 if (cfg->autoStartBypassCache)
@@ -234,7 +234,6 @@ qemuAutostartDomain(virDomainObjPtr vm,
 ret = 0;
  cleanup:
 virDomainObjEndAPI(&vm);
-virObjectUnref(cfg);
 return ret;
 }
 
@@ -308,7 +307,7 @@ qemuSecurityInit(virQEMUDriverPtr driver)
 char **names;
 virSecurityManagerPtr mgr = NULL;
 virSecurityManagerPtr stack = NULL;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 unsigned int flags = 0;
 
 if (cfg->securityDefaultConfined)
@@ -368,7 +367,6 @@ qemuSecurityInit(virQEMUDriverPtr driver)
 }
 
 driver->securityManager = stack;
-virObjectUnref(cfg);
 return 0;
 
  error:
@@ -376,7 +374,6 @@ qemuSecurityInit(virQEMUDriverPtr driver)
_("Failed to initialize security drivers"));
 virObjectUnref(stack);
 virObjectUnref(mgr);
-virObjectUnref(cfg);
 return -1;
 }
 
@@ -1074,7 +1071,7 @@ static void qemuNotifyLoadDomain(virDomainObjPtr vm, int 
newVM, void *opaque)
 static int
 qemuStateReload(void)
 {
-virQEMUDriverConfigPtr cfg = NULL;
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
 virCapsPtr caps = NULL;
 
 if (!qemu_driver)
@@ -1090,7 +1087,6 @@ qemuStateReload(void)
caps, qemu_driver->xmlopt,
qemuNotifyLoadDomain, qemu_driver);
  cleanup:
-virObjectUnref(cfg);
 virObjectUnref(caps);
 return 0;
 }
@@ -1112,7 +1108,7 @@ qemuStateStop(void)
 int state;
 virDomainPtr *domains = NULL;
 unsigned int *flags = NULL;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(qemu_driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = 
virQEMUDriverGetConfig(qemu_driver);
 
 if (!(conn = virConnectOpen(cfg->uri)))
 goto cleanup;
@@ -1151,7 +1147,6 @@ qemuStateStop(void)
 }
 VIR_FREE(flags);
 virObjectUnref(conn);
-virObjectUnref(cfg);
 
 return ret;
 }
@@ -1209,20 +1204,16 @@ qemuStateCleanup(void)
 static int
 qemuConnectURIProbe(char **uri)
 {
-virQEMUDriverConfigPtr cfg = NULL;
-int ret = -1;
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
 
 if (qemu_driver == NULL)
 return 0;
 
 cfg = virQEMUDriverGetConfig(qemu_driver);
 if (VIR_STRDUP(*uri, cfg->uri) < 0)
-goto cleanup;
+return -1;
 
-ret = 0;
- cleanup:
-virObjectUnref(cfg);
-return ret;
+return 0;
 }
 
 static virDrvOpenStatus qemuConnectOpen(virConnectPtr conn,
@@ -1887,7 +1878,7 @@ static int qemuDomainSuspend(virDomainPtr dom)
 qemuDomainObjPrivatePtr priv;
 virDomainPausedReason reason;
 int state;
-virQEMUDriverConfigPtr cfg = NULL;
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
 
 if (!(vm = qemuDomainObjFromDomain(dom)))
 return -1;
@@ -1930,7 +1921,6 @@ static int qemuDomainSuspend(virDomainPtr dom)
  cleanup:
 virDomainObjEndAPI(&vm);
 
-virObjectUnref(cfg);
 return ret;
 }
 
@@ -1942,7 +1932,7 @@ static int qemuDomainResume(virDomainPtr dom)
 int ret = -1;
 int state;
 int reason;
-virQEMUDriverConfigPtr cfg = NULL;
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
 
 if (!(vm = qemuDomainObjFromDomain(dom)))
 return -1;
@@ -1988,7 +1978,6 @@ static int qemuDomainResume(virDomainPtr dom)
 
  cleanup:
 virDomainObjEndAPI(&vm);
-virObjectUnref(cfg);
 return ret;
 }
 
@@ -2369,7 +2358,7 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, 
unsigned long newmem,
 virDomainDefPtr def;
 virDomainDefPtr persistentDef;
 int ret = -1, r;
-virQEMUDriverConfigPtr cfg = NULL;
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
 
 virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
   VIR_DOMAIN_AFFECT_CONFIG |
@@ -2474,7 +2463,6 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, 
unsigned long newmem,
 
  cleanup:
 virDomainObjEndAPI(&vm);
-virObjectUnref(cf

[libvirt] [PATCH v3 06/10] qemu_driver: use VIR_AUTOFREE() with strings 1/3

[Daniel Henrique Barboza]

Using VIR_AUTOFREE() in all strings of qemu_driver.c make the code
a bit tidier and smaller, sparing VIR_FREE() calls and sometimes a
whole 'cleanup' label.

This is a huge change due to the amount of char * declared in
this file, thus let's split it in 3. This is the first part.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_driver.c | 103 +
 1 file changed, 33 insertions(+), 70 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d95c5c5b81..f887a79ecd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -381,11 +381,9 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
void *data)
 {
 char *baseDir = (char *)data;
-char *snapDir = NULL;
+VIR_AUTOFREE(char *) snapDir = NULL;
 DIR *dir = NULL;
 struct dirent *entry;
-char *xmlStr;
-char *fullpath;
 virDomainSnapshotDefPtr def = NULL;
 virDomainMomentObjPtr snap = NULL;
 virDomainMomentObjPtr current = NULL;
@@ -420,6 +418,9 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
 goto cleanup;
 
 while ((direrr = virDirRead(dir, &entry, NULL)) > 0) {
+VIR_AUTOFREE(char *) xmlStr = NULL;
+VIR_AUTOFREE(char *) fullpath = NULL;
+
 /* NB: ignoring errors, so one malformed config doesn't
kill the whole process */
 VIR_INFO("Loading snapshot file '%s'", entry->d_name);
@@ -435,7 +436,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
 virReportSystemError(errno,
  _("Failed to read snapshot file %s"),
  fullpath);
-VIR_FREE(fullpath);
 continue;
 }
 
@@ -448,8 +448,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to parse snapshot XML from file '%s'"),
fullpath);
-VIR_FREE(fullpath);
-VIR_FREE(xmlStr);
 continue;
 }
 
@@ -463,9 +461,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
vm->def->name);
 current = snap;
 }
-
-VIR_FREE(fullpath);
-VIR_FREE(xmlStr);
 }
 if (direrr < 0)
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -492,7 +487,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
 ret = 0;
  cleanup:
 VIR_DIR_CLOSE(dir);
-VIR_FREE(snapDir);
 virObjectUnlock(vm);
 return ret;
 }
@@ -503,11 +497,9 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm,
  void *data)
 {
 char *baseDir = (char *)data;
-char *chkDir = NULL;
+VIR_AUTOFREE(char *) chkDir = NULL;
 DIR *dir = NULL;
 struct dirent *entry;
-char *xmlStr;
-char *fullpath;
 virDomainCheckpointDefPtr def = NULL;
 virDomainMomentObjPtr chk = NULL;
 virDomainMomentObjPtr current = NULL;
@@ -538,6 +530,9 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm,
 goto cleanup;
 
 while ((direrr = virDirRead(dir, &entry, NULL)) > 0) {
+VIR_AUTOFREE(char *) xmlStr = NULL;
+VIR_AUTOFREE(char *) fullpath = NULL;
+
 /* NB: ignoring errors, so one malformed config doesn't
kill the whole process */
 VIR_INFO("Loading checkpoint file '%s'", entry->d_name);
@@ -553,7 +548,6 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm,
 virReportSystemError(errno,
  _("Failed to read checkpoint file %s"),
  fullpath);
-VIR_FREE(fullpath);
 continue;
 }
 
@@ -566,8 +560,6 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm,
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to parse checkpoint XML from file '%s'"),
fullpath);
-VIR_FREE(fullpath);
-VIR_FREE(xmlStr);
 virObjectUnref(def);
 continue;
 }
@@ -575,9 +567,6 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm,
 chk = virDomainCheckpointAssignDef(vm->checkpoints, def);
 if (chk == NULL)
 virObjectUnref(def);
-
-VIR_FREE(fullpath);
-VIR_FREE(xmlStr);
 }
 if (direrr < 0)
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -602,7 +591,6 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm,
 ret = 0;
  cleanup:
 VIR_DIR_CLOSE(dir);
-VIR_FREE(chkDir);
 virObjectUnlock(vm);
 return ret;
 }
@@ -660,12 +648,11 @@ qemuStateInitialize(bool privileged,
 virStateInhibitCallback callback,
 void *opaque)
 {
-char *driverConf = NULL;
+VIR_AUTOFREE(char *) driverConf = NULL;
 virQEMUDriverConfigPtr cfg;
 uid_t run_uid = -1;
 gid_t run_gid = -1;
-char *hugepagePath = NULL;
-char *memoryBackingPath = NULL;
+VIR_AUTOFREE(char *) memoryBackingPath = NULL;
 bool autostart = true;
 siz

[libvirt] [PATCH v3 07/10] qemu_driver: use VIR_AUTOFREE() with strings 2/3

[Daniel Henrique Barboza]

Using VIR_AUTOFREE() in all strings of qemu_driver.c make the code
a bit tidier and smaller, sparing VIR_FREE() calls and sometimes a
whole 'cleanup' label.

This is a huge change due to the amount of char * declared in
this file, thus let's split it in 3. This is the second part.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_driver.c | 86 +++---
 1 file changed, 30 insertions(+), 56 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f887a79ecd..f3fccc0843 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4108,11 +4108,11 @@ processWatchdogEvent(virQEMUDriverPtr driver,
 {
 int ret;
 VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
-char *dumpfile = getAutoDumpPath(driver, vm);
+VIR_AUTOFREE(char *) dumpfile = getAutoDumpPath(driver, vm);
 unsigned int flags = VIR_DUMP_MEMORY_ONLY;
 
 if (!dumpfile)
-goto cleanup;
+return;
 
 switch (action) {
 case VIR_DOMAIN_WATCHDOG_ACTION_DUMP:
@@ -4120,7 +4120,7 @@ processWatchdogEvent(virQEMUDriverPtr driver,
QEMU_ASYNC_JOB_DUMP,
VIR_DOMAIN_JOB_OPERATION_DUMP,
flags) < 0) {
-goto cleanup;
+return;
 }
 
 if (virDomainObjCheckActive(vm) < 0)
@@ -4141,14 +4141,11 @@ processWatchdogEvent(virQEMUDriverPtr driver,
"%s", _("Resuming after dump failed"));
 break;
 default:
-goto cleanup;
+return;
 }
 
  endjob:
 qemuDomainObjEndAsyncJob(driver, vm);
-
- cleanup:
-VIR_FREE(dumpfile);
 }
 
 static int
@@ -4158,18 +4155,16 @@ doCoreDumpToAutoDumpPath(virQEMUDriverPtr driver,
 {
 int ret = -1;
 VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
-char *dumpfile = getAutoDumpPath(driver, vm);
+VIR_AUTOFREE(char *) dumpfile = getAutoDumpPath(driver, vm);
 
 if (!dumpfile)
-goto cleanup;
+return -1;
 
 flags |= cfg->autoDumpBypassCache ? VIR_DUMP_BYPASS_CACHE: 0;
 if ((ret = doCoreDump(driver, vm, dumpfile, flags,
   VIR_DOMAIN_CORE_DUMP_FORMAT_RAW)) < 0)
 virReportError(VIR_ERR_OPERATION_FAILED,
"%s", _("Dump failed"));
- cleanup:
-VIR_FREE(dumpfile);
 return ret;
 }
 
@@ -4179,14 +4174,11 @@ qemuProcessGuestPanicEventInfo(virQEMUDriverPtr driver,
virDomainObjPtr vm,
qemuMonitorEventPanicInfoPtr info)
 {
-char *msg = qemuMonitorGuestPanicEventInfoFormatMsg(info);
-char *timestamp = virTimeStringNow();
+VIR_AUTOFREE(char *) msg = qemuMonitorGuestPanicEventInfoFormatMsg(info);
+VIR_AUTOFREE(char *) timestamp = virTimeStringNow();
 
 if (msg && timestamp)
 qemuDomainLogAppendMessage(driver, vm, "%s: panic %s\n", timestamp, 
msg);
-
-VIR_FREE(timestamp);
-VIR_FREE(msg);
 }
 
 
@@ -5082,7 +5074,7 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm,
 virDomainVcpuDefPtr vcpuinfo;
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virCgroupPtr cgroup_vcpu = NULL;
-char *str = NULL;
+VIR_AUTOFREE(char *) str = NULL;
 virObjectEventPtr event = NULL;
 char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = "";
 virTypedParameterPtr eventParams = NULL;
@@ -5146,7 +5138,6 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm,
  cleanup:
 virBitmapFree(tmpmap);
 virCgroupFree(&cgroup_vcpu);
-VIR_FREE(str);
 virObjectEventStateQueue(driver->domainEventState, event);
 return ret;
 }
@@ -5287,7 +5278,7 @@ qemuDomainPinEmulator(virDomainPtr dom,
 virBitmapPtr pcpumap = NULL;
 VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
 virObjectEventPtr event = NULL;
-char *str = NULL;
+VIR_AUTOFREE(char *) str = NULL;
 virTypedParameterPtr eventParams = NULL;
 int eventNparams = 0;
 int eventMaxparams = 0;
@@ -5376,7 +5367,6 @@ qemuDomainPinEmulator(virDomainPtr dom,
 if (cgroup_emulator)
 virCgroupFree(&cgroup_emulator);
 virObjectEventStateQueue(driver->domainEventState, event);
-VIR_FREE(str);
 virBitmapFree(pcpumap);
 virDomainObjEndAPI(&vm);
 return ret;
@@ -5758,7 +5748,7 @@ qemuDomainPinIOThread(virDomainPtr dom,
 virCgroupPtr cgroup_iothread = NULL;
 virObjectEventPtr event = NULL;
 char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = "";
-char *str = NULL;
+VIR_AUTOFREE(char *) str = NULL;
 virTypedParameterPtr eventParams = NULL;
 int eventNparams = 0;
 int eventMaxparams = 0;
@@ -5870,7 +5860,6 @@ qemuDomainPinIOThread(virDomainPtr dom,
 if (cgroup_iothread)
 virCgroupFree(&cgroup_iothread);
 virObjectEventStateQueue(driver->domainEventState, event);
-VIR_FREE(str);
 virBitmapFree(pcpumap);
 virDomainObjEndAPI(&vm);
 return ret;

[libvirt] [PATCH v3 10/10] qemu_driver.c: use GLib macros

[Daniel Henrique Barboza]

Previous patches added VIR_AUTO* facilities in qemu_driver.c to
cleanup virObjectUnref() and VIR_FREE() calls. This patch now
converts those VIR_AUTO* macros to their GLib alternatives,
g_autoptr and g_autofree.

After this patch, VIR_AUTO* macros are being used only in three
functions:

- qemuSecurityInit
- qemuDomainSaveInternal
- qemuDomainSaveImageStartVM

The rest of the file was converted to GLib macros.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_driver.c | 366 -
 1 file changed, 183 insertions(+), 183 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6f283d2de3..046f68ad40 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -202,7 +202,7 @@ qemuAutostartDomain(virDomainObjPtr vm,
 {
 virQEMUDriverPtr driver = opaque;
 int flags = 0;
-VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 int ret = -1;
 
 if (cfg->autoStartBypassCache)
@@ -254,7 +254,7 @@ qemuSecurityChownCallback(const virStorageSource *src,
 int save_errno = 0;
 int ret = -1;
 int rv;
-VIR_AUTOUNREF(virStorageSourcePtr) cpy = NULL;
+g_autoptr(virStorageSource) cpy = NULL;
 
 rv = virStorageFileSupportsSecurityDriver(src);
 if (rv <= 0)
@@ -381,7 +381,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
void *data)
 {
 char *baseDir = (char *)data;
-VIR_AUTOFREE(char *) snapDir = NULL;
+g_autofree char *snapDir = NULL;
 DIR *dir = NULL;
 struct dirent *entry;
 virDomainSnapshotDefPtr def = NULL;
@@ -392,7 +392,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
   VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
   VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL);
 int ret = -1;
-VIR_AUTOUNREF(virCapsPtr) caps = NULL;
+g_autoptr(virCaps) caps = NULL;
 int direrr;
 qemuDomainObjPrivatePtr priv;
 
@@ -418,8 +418,8 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
 goto cleanup;
 
 while ((direrr = virDirRead(dir, &entry, NULL)) > 0) {
-VIR_AUTOFREE(char *) xmlStr = NULL;
-VIR_AUTOFREE(char *) fullpath = NULL;
+g_autofree char *xmlStr = NULL;
+g_autofree char *fullpath = NULL;
 
 /* NB: ignoring errors, so one malformed config doesn't
kill the whole process */
@@ -497,7 +497,7 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm,
  void *data)
 {
 char *baseDir = (char *)data;
-VIR_AUTOFREE(char *) chkDir = NULL;
+g_autofree char *chkDir = NULL;
 DIR *dir = NULL;
 struct dirent *entry;
 virDomainCheckpointDefPtr def = NULL;
@@ -505,7 +505,7 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm,
 virDomainMomentObjPtr current = NULL;
 unsigned int flags = VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE;
 int ret = -1;
-VIR_AUTOUNREF(virCapsPtr) caps = NULL;
+g_autoptr(virCaps) caps = NULL;
 int direrr;
 qemuDomainObjPrivatePtr priv;
 
@@ -530,8 +530,8 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm,
 goto cleanup;
 
 while ((direrr = virDirRead(dir, &entry, NULL)) > 0) {
-VIR_AUTOFREE(char *) xmlStr = NULL;
-VIR_AUTOFREE(char *) fullpath = NULL;
+g_autofree char *xmlStr = NULL;
+g_autofree char *fullpath = NULL;
 
 /* NB: ignoring errors, so one malformed config doesn't
kill the whole process */
@@ -648,11 +648,11 @@ qemuStateInitialize(bool privileged,
 virStateInhibitCallback callback,
 void *opaque)
 {
-VIR_AUTOFREE(char *) driverConf = NULL;
+g_autofree char *driverConf = NULL;
 virQEMUDriverConfigPtr cfg;
 uid_t run_uid = -1;
 gid_t run_gid = -1;
-VIR_AUTOFREE(char *) memoryBackingPath = NULL;
+g_autofree char *memoryBackingPath = NULL;
 bool autostart = true;
 size_t i;
 
@@ -816,7 +816,7 @@ qemuStateInitialize(bool privileged,
 goto error;
 
 if (privileged) {
-VIR_AUTOFREE(char *) channeldir = NULL;
+g_autofree char *channeldir = NULL;
 
 if (chown(cfg->libDir, cfg->user, cfg->group) < 0) {
 virReportSystemError(errno,
@@ -921,7 +921,7 @@ qemuStateInitialize(bool privileged,
  * it, since we can't assume the root mount point has permissions that
  * will let our spawned QEMU instances use it. */
 for (i = 0; i < cfg->nhugetlbfs; i++) {
-VIR_AUTOFREE(char *) hugepagePath = NULL;
+g_autofree char *hugepagePath = NULL;
 
 hugepagePath = qemuGetBaseHugepagePath(&cfg->hugetlbfs[i]);
 
@@ -1048,8 +1048,8 @@ static void qemuNotifyLoadDomain(virDomainObjPtr vm, int 
newVM, void *opaque)
 static int
 qemuStateReload(void)
 {
-VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
-VIR_AUTOUNREF(virCapsPtr) caps = NULL;
+g_autoptr(virQEMUDriverConfig) cfg = NULL;
+g_autoptr(virC

[libvirt] [PATCH v3 08/10] qemu_driver: use VIR_AUTOFREE() with strings 3/3

[Daniel Henrique Barboza]

Using VIR_AUTOFREE() in all strings of qemu_driver.c make the code
a bit tidier and smaller, sparing VIR_FREE() calls and sometimes a
whole 'cleanup' label.

This is the last part of this change.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_driver.c | 99 +++---
 1 file changed, 34 insertions(+), 65 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f3fccc0843..fd7c8bb1be 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11956,7 +11956,7 @@ qemuDomainMemoryPeek(virDomainPtr dom,
 {
 virQEMUDriverPtr driver = dom->conn->privateData;
 virDomainObjPtr vm;
-char *tmp = NULL;
+VIR_AUTOFREE(char *) tmp = NULL;
 int fd = -1, ret = -1;
 qemuDomainObjPrivatePtr priv;
 VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
@@ -12028,7 +12028,6 @@ qemuDomainMemoryPeek(virDomainPtr dom,
 VIR_FORCE_CLOSE(fd);
 if (tmp)
 unlink(tmp);
-VIR_FREE(tmp);
 virDomainObjEndAPI(&vm);
 return ret;
 }
@@ -12194,7 +12193,7 @@ qemuStorageLimitsRefresh(virQEMUDriverPtr driver,
 int ret = -1;
 int fd = -1;
 struct stat sb;
-char *buf = NULL;
+VIR_AUTOFREE(char *) buf = NULL;
 ssize_t len;
 
 if ((rc = qemuDomainStorageOpenStat(driver, cfg, vm, src, &fd, &sb,
@@ -12230,7 +12229,6 @@ qemuStorageLimitsRefresh(virQEMUDriverPtr driver,
 ret = 1;
 
  cleanup:
-VIR_FREE(buf);
 qemuDomainStorageCloseStat(src, &fd);
 return ret;
 }
@@ -12451,7 +12449,7 @@ qemuDomainMigratePrepareTunnel(virConnectPtr dconn,
 {
 virQEMUDriverPtr driver = dconn->privateData;
 virDomainDefPtr def = NULL;
-char *origname = NULL;
+VIR_AUTOFREE(char *) origname = NULL;
 qemuMigrationParamsPtr migParams = NULL;
 int ret = -1;
 
@@ -12486,7 +12484,6 @@ qemuDomainMigratePrepareTunnel(virConnectPtr dconn,
 
  cleanup:
 qemuMigrationParamsFree(migParams);
-VIR_FREE(origname);
 virDomainDefFree(def);
 return ret;
 }
@@ -12508,7 +12505,7 @@ qemuDomainMigratePrepare2(virConnectPtr dconn,
 {
 virQEMUDriverPtr driver = dconn->privateData;
 virDomainDefPtr def = NULL;
-char *origname = NULL;
+VIR_AUTOFREE(char *) origname = NULL;
 qemuMigrationParamsPtr migParams = NULL;
 int ret = -1;
 
@@ -12553,7 +12550,6 @@ qemuDomainMigratePrepare2(virConnectPtr dconn,
 
  cleanup:
 qemuMigrationParamsFree(migParams);
-VIR_FREE(origname);
 virDomainDefFree(def);
 return ret;
 }
@@ -12754,7 +12750,7 @@ qemuDomainMigratePrepare3(virConnectPtr dconn,
 {
 virQEMUDriverPtr driver = dconn->privateData;
 virDomainDefPtr def = NULL;
-char *origname = NULL;
+VIR_AUTOFREE(char *) origname = NULL;
 qemuMigrationParamsPtr migParams = NULL;
 int ret = -1;
 
@@ -12789,7 +12785,6 @@ qemuDomainMigratePrepare3(virConnectPtr dconn,
 
  cleanup:
 qemuMigrationParamsFree(migParams);
-VIR_FREE(origname);
 virDomainDefFree(def);
 return ret;
 }
@@ -12815,7 +12810,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn,
 int nbdPort = 0;
 int nmigrate_disks;
 const char **migrate_disks = NULL;
-char *origname = NULL;
+VIR_AUTOFREE(char *) origname = NULL;
 qemuMigrationParamsPtr migParams = NULL;
 int ret = -1;
 
@@ -12878,7 +12873,6 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn,
  cleanup:
 qemuMigrationParamsFree(migParams);
 VIR_FREE(migrate_disks);
-VIR_FREE(origname);
 virDomainDefFree(def);
 return ret;
 }
@@ -12898,7 +12892,7 @@ qemuDomainMigratePrepareTunnel3(virConnectPtr dconn,
 {
 virQEMUDriverPtr driver = dconn->privateData;
 virDomainDefPtr def = NULL;
-char *origname = NULL;
+VIR_AUTOFREE(char *) origname = NULL;
 qemuMigrationParamsPtr migParams = NULL;
 int ret = -1;
 
@@ -12927,7 +12921,6 @@ qemuDomainMigratePrepareTunnel3(virConnectPtr dconn,
 
  cleanup:
 qemuMigrationParamsFree(migParams);
-VIR_FREE(origname);
 virDomainDefFree(def);
 return ret;
 }
@@ -12947,7 +12940,7 @@ qemuDomainMigratePrepareTunnel3Params(virConnectPtr 
dconn,
 virDomainDefPtr def = NULL;
 const char *dom_xml = NULL;
 const char *dname = NULL;
-char *origname = NULL;
+VIR_AUTOFREE(char *) origname = NULL;
 qemuMigrationParamsPtr migParams = NULL;
 int ret = -1;
 
@@ -12986,7 +12979,6 @@ qemuDomainMigratePrepareTunnel3Params(virConnectPtr 
dconn,
 
  cleanup:
 qemuMigrationParamsFree(migParams);
-VIR_FREE(origname);
 virDomainDefFree(def);
 return ret;
 }
@@ -13308,7 +13300,7 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev,
 unsigned domain = 0, bus = 0, slot = 0, function = 0;
 int ret = -1;
 virNodeDeviceDefPtr def = NULL;
-char *xml = NULL;
+VIR_AUTOFREE(char *) xml = NULL;
 bool vfio = qemuHostdevHostSupportsPassthroughVFIO();
 virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
 
@@ -13358,7 +13350,6 @@ qemuNodeDeviceDetachFlags(virNodeD

[libvirt] [PATCH v3 05/10] qemu_driver: use VIR_AUTOUNREF() with more pointer types

[Daniel Henrique Barboza]

This patch uses VIR_AUTOUNREF() with the following pointer types:

- virQEMUCapsPtr
- virConnect
- qemuDomainSaveCookiePtr
- virDomainCapsPtr
- qemuBlockJobDataPtr*
- virNetworkPtr
- virSecurityManagerPtr

'cleanup' labels were deleted when possible.

* instances being cleaned up with qemuBlockJobStartupFinalize()
weren't changed, since qemuBlockJobStartupFinalize() will
unref the object after qemuBlockJobUnregister().

Suggested-by: Erik Skultety 
Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_driver.c | 64 +++---
 1 file changed, 22 insertions(+), 42 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 99923b0ab6..d95c5c5b81 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -305,8 +305,8 @@ static int
 qemuSecurityInit(virQEMUDriverPtr driver)
 {
 char **names;
-virSecurityManagerPtr mgr = NULL;
-virSecurityManagerPtr stack = NULL;
+VIR_AUTOUNREF(virSecurityManagerPtr) mgr = NULL;
+VIR_AUTOUNREF(virSecurityManagerPtr) stack = NULL;
 VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 unsigned int flags = 0;
 
@@ -372,8 +372,6 @@ qemuSecurityInit(virQEMUDriverPtr driver)
  error:
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to initialize security drivers"));
-virObjectUnref(stack);
-virObjectUnref(mgr);
 return -1;
 }
 
@@ -1098,7 +1096,7 @@ static int
 qemuStateStop(void)
 {
 int ret = -1;
-virConnectPtr conn;
+VIR_AUTOUNREF(virConnectPtr) conn = NULL;
 int numDomains = 0;
 size_t i;
 int state;
@@ -1142,7 +1140,6 @@ qemuStateStop(void)
 VIR_FREE(domains);
 }
 VIR_FREE(flags);
-virObjectUnref(conn);
 
 return ret;
 }
@@ -3310,7 +3307,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver,
 qemuDomainObjPrivatePtr priv = vm->privateData;
 VIR_AUTOUNREF(virCapsPtr) caps = NULL;
 virQEMUSaveDataPtr data = NULL;
-qemuDomainSaveCookiePtr cookie = NULL;
+VIR_AUTOUNREF(qemuDomainSaveCookiePtr) cookie = NULL;
 
 if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
 goto cleanup;
@@ -3419,7 +3416,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver,
 qemuDomainRemoveInactiveJob(driver, vm);
 
  cleanup:
-virObjectUnref(cookie);
 VIR_FREE(xml);
 virQEMUSaveDataFree(data);
 virObjectEventStateQueue(driver->domainEventState, event);
@@ -6866,7 +6862,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
 char *errbuf = NULL;
 VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 virQEMUSaveHeaderPtr header = &data->header;
-qemuDomainSaveCookiePtr cookie = NULL;
+VIR_AUTOUNREF(qemuDomainSaveCookiePtr) cookie = NULL;
 
 if (virSaveCookieParseString(data->cookie, (virObjectPtr *)&cookie,
  
virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0)
@@ -6981,7 +6977,6 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
 ret = 0;
 
  cleanup:
-virObjectUnref(cookie);
 virCommandFree(cmd);
 VIR_FREE(errbuf);
 if (qemuSecurityRestoreSavedStateLabel(driver, vm, path) < 0)
@@ -13588,7 +13583,7 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn,
 int ret = VIR_CPU_COMPARE_ERROR;
 virQEMUDriverPtr driver = conn->privateData;
 VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
-virQEMUCapsPtr qemuCaps = NULL;
+VIR_AUTOUNREF(virQEMUCapsPtr) qemuCaps = NULL;
 bool failIncompatible;
 virCPUDefPtr hvCPU;
 virCPUDefPtr cpu = NULL;
@@ -13642,7 +13637,6 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn,
 
  cleanup:
 virCPUDefFree(cpu);
-virObjectUnref(qemuCaps);
 return ret;
 }
 
@@ -13802,7 +13796,7 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
 virQEMUDriverPtr driver = conn->privateData;
 VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 virCPUDefPtr *cpus = NULL;
-virQEMUCapsPtr qemuCaps = NULL;
+VIR_AUTOUNREF(virQEMUCapsPtr) qemuCaps = NULL;
 virArch arch;
 virDomainVirtType virttype;
 virDomainCapsCPUModelsPtr cpuModels;
@@ -13881,7 +13875,6 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn,
  cleanup:
 virCPUDefListFree(cpus);
 virCPUDefFree(cpu);
-virObjectUnref(qemuCaps);
 virStringListFree(features);
 
 return cpustr;
@@ -17689,7 +17682,7 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
 VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 bool pivot = !!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT);
 bool async = !!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC);
-qemuBlockJobDataPtr job = NULL;
+VIR_AUTOUNREF(qemuBlockJobDataPtr) job = NULL;
 virDomainObjPtr vm;
 qemuDomainObjPrivatePtr priv = NULL;
 bool blockdev = false;
@@ -17790,7 +17783,6 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
 qemuDomainObjEndJob(driver, vm);
 
  

[libvirt] [PATCH v3 03/10] qemu_driver: use VIR_AUTOUNREF() with virQEMUDriverConfigPtr 3/3

[Daniel Henrique Barboza]

virQEMUDriverConfigPtr can be auto-unref for the great majority
of the uses made in qemu_driver, sparing us a virObjectUnref()
call and sometimes a whole 'cleanup' label.

This patch changes virQEMUDriverConfigPtr declarations to
use VIR_AUTOUNREF(). 'cleanup' labels were deleted when
applicable.

This is the last part of this change. All but one* instance of
virQEMUDriverConfigPtr were changed to use VIR_AUTOUNREF().
'cleanup' labels were deleted when applicable.

* qemuStateInitialize: we can't auto-unref the pointer since we're
initializing the qemu_driver object with it.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_driver.c | 51 ++
 1 file changed, 17 insertions(+), 34 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9f9443f493..d9effd7f34 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10150,7 +10150,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
 virDomainDefPtr persistentDef;
 virDomainObjPtr vm = NULL;
 int ret = -1;
-virQEMUDriverConfigPtr cfg = NULL;
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
 qemuDomainObjPrivatePtr priv;
 virBitmapPtr nodeset = NULL;
 virDomainNumatuneMemMode config_mode;
@@ -10262,7 +10262,6 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
  cleanup:
 virBitmapFree(nodeset);
 virDomainObjEndAPI(&vm);
-virObjectUnref(cfg);
 return ret;
 }
 
@@ -10367,7 +10366,7 @@ qemuDomainSetPerfEvents(virDomainPtr dom,
 virQEMUDriverPtr driver = dom->conn->privateData;
 size_t i;
 virDomainObjPtr vm = NULL;
-virQEMUDriverConfigPtr cfg = NULL;
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
 qemuDomainObjPrivatePtr priv;
 virDomainDefPtr def;
 virDomainDefPtr persistentDef;
@@ -10459,7 +10458,6 @@ qemuDomainSetPerfEvents(virDomainPtr dom,
 
  cleanup:
 virDomainObjEndAPI(&vm);
-virObjectUnref(cfg);
 return ret;
 }
 
@@ -10646,7 +10644,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
 long long value_l;
 int ret = -1;
 int rc;
-virQEMUDriverConfigPtr cfg = NULL;
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
 virCapsPtr caps = NULL;
 qemuDomainObjPrivatePtr priv;
 virObjectEventPtr event = NULL;
@@ -10940,7 +10938,6 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
 if (eventNparams)
 virTypedParamsFree(eventParams, eventNparams);
 virObjectUnref(caps);
-virObjectUnref(cfg);
 return ret;
 }
 #undef SCHED_RANGE_CHECK
@@ -11605,7 +11602,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
 int ret = -1;
 virDomainNetDefPtr net = NULL, persistentNet = NULL;
 virNetDevBandwidthPtr bandwidth = NULL, newBandwidth = NULL;
-virQEMUDriverConfigPtr cfg = NULL;
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
 bool inboundSpecified = false, outboundSpecified = false;
 int actualType;
 bool qosSupported = true;
@@ -11800,7 +11797,6 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
 virNetDevBandwidthFree(bandwidth);
 virNetDevBandwidthFree(newBandwidth);
 virDomainObjEndAPI(&vm);
-virObjectUnref(cfg);
 return ret;
 }
 
@@ -12062,7 +12058,7 @@ qemuDomainMemoryPeek(virDomainPtr dom,
 char *tmp = NULL;
 int fd = -1, ret = -1;
 qemuDomainObjPrivatePtr priv;
-virQEMUDriverConfigPtr cfg = NULL;
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
 
 virCheckFlags(VIR_MEMORY_VIRTUAL | VIR_MEMORY_PHYSICAL, -1);
 
@@ -12133,7 +12129,6 @@ qemuDomainMemoryPeek(virDomainPtr dom,
 unlink(tmp);
 VIR_FREE(tmp);
 virDomainObjEndAPI(&vm);
-virObjectUnref(cfg);
 return ret;
 }
 
@@ -12350,7 +12345,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
 virDomainObjPtr vm;
 int ret = -1;
 virDomainDiskDefPtr disk;
-virQEMUDriverConfigPtr cfg = NULL;
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
 qemuBlockStatsPtr entry = NULL;
 
 virCheckFlags(0, -1);
@@ -12439,7 +12434,6 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
  cleanup:
 VIR_FREE(entry);
 virDomainObjEndAPI(&vm);
-virObjectUnref(cfg);
 return ret;
 }
 
@@ -12911,7 +12905,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn,
 unsigned int flags)
 {
 virQEMUDriverPtr driver = dconn->privateData;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 virDomainDefPtr def = NULL;
 const char *dom_xml = NULL;
 const char *dname = NULL;
@@ -12985,7 +12979,6 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn,
 VIR_FREE(migrate_disks);
 VIR_FREE(origname);
 virDomainDefFree(def);
-virObjectUnref(cfg);
 return ret;
 }
 
@@ -14709,7 +14702,7 @@ 
qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver,
 virCommandPtr cmd = NULL;
 const char *qemuImgPath;
 vir

[libvirt] [PATCH v3 04/10] qemu_driver: use VIR_AUTOUNREF() with virCapsPtr

[Daniel Henrique Barboza]

virCapsPtr can be auto-unref for the great majority
of the uses made in qemu_driver, sparing us a virObjectUnref()
call and sometimes a whole 'cleanup' label.

This patch changes virCapsPtr declarations to use VIR_AUTOUNREF().
'cleanup' labels were deleted when possible.

Suggested-by: Erik Skultety 
Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_driver.c | 113 ++---
 1 file changed, 37 insertions(+), 76 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d9effd7f34..99923b0ab6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -396,7 +396,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
   VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
   VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL);
 int ret = -1;
-virCapsPtr caps = NULL;
+VIR_AUTOUNREF(virCapsPtr) caps = NULL;
 int direrr;
 qemuDomainObjPrivatePtr priv;
 
@@ -495,7 +495,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
  cleanup:
 VIR_DIR_CLOSE(dir);
 VIR_FREE(snapDir);
-virObjectUnref(caps);
 virObjectUnlock(vm);
 return ret;
 }
@@ -516,7 +515,7 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm,
 virDomainMomentObjPtr current = NULL;
 unsigned int flags = VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE;
 int ret = -1;
-virCapsPtr caps = NULL;
+VIR_AUTOUNREF(virCapsPtr) caps = NULL;
 int direrr;
 qemuDomainObjPrivatePtr priv;
 
@@ -606,7 +605,6 @@ qemuDomainCheckpointLoad(virDomainObjPtr vm,
  cleanup:
 VIR_DIR_CLOSE(dir);
 VIR_FREE(chkDir);
-virObjectUnref(caps);
 virObjectUnlock(vm);
 return ret;
 }
@@ -1072,13 +1070,13 @@ static int
 qemuStateReload(void)
 {
 VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL;
-virCapsPtr caps = NULL;
+VIR_AUTOUNREF(virCapsPtr) caps = NULL;
 
 if (!qemu_driver)
 return 0;
 
 if (!(caps = virQEMUDriverGetCapabilities(qemu_driver, false)))
-goto cleanup;
+return 0;
 
 cfg = virQEMUDriverGetConfig(qemu_driver);
 virDomainObjListLoadAllConfigs(qemu_driver->domains,
@@ -1086,8 +1084,6 @@ qemuStateReload(void)
cfg->autostartDir, false,
caps, qemu_driver->xmlopt,
qemuNotifyLoadDomain, qemu_driver);
- cleanup:
-virObjectUnref(caps);
 return 0;
 }
 
@@ -1356,21 +1352,15 @@ qemuConnectGetMaxVcpus(virConnectPtr conn 
G_GNUC_UNUSED, const char *type)
 
 static char *qemuConnectGetCapabilities(virConnectPtr conn) {
 virQEMUDriverPtr driver = conn->privateData;
-virCapsPtr caps = NULL;
-char *xml = NULL;
+VIR_AUTOUNREF(virCapsPtr) caps = NULL;
 
 if (virConnectGetCapabilitiesEnsureACL(conn) < 0)
 return NULL;
 
 if (!(caps = virQEMUDriverGetCapabilities(driver, true)))
-goto cleanup;
-
-xml = virCapabilitiesFormatXML(caps);
-virObjectUnref(caps);
-
- cleanup:
+return NULL;
 
-return xml;
+return virCapabilitiesFormatXML(caps);
 }
 
 
@@ -1716,27 +1706,22 @@ static int qemuDomainIsUpdated(virDomainPtr dom)
 static int qemuConnectGetVersion(virConnectPtr conn, unsigned long *version)
 {
 virQEMUDriverPtr driver = conn->privateData;
-int ret = -1;
 unsigned int qemuVersion = 0;
-virCapsPtr caps = NULL;
+VIR_AUTOUNREF(virCapsPtr) caps = NULL;
 
 if (virConnectGetVersionEnsureACL(conn) < 0)
 return -1;
 
 if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
-goto cleanup;
+return -1;
 
 if (virQEMUCapsGetDefaultVersion(caps,
  driver->qemuCapsCache,
  &qemuVersion) < 0)
-goto cleanup;
+return -1;
 
 *version = qemuVersion;
-ret = 0;
-
- cleanup:
-virObjectUnref(caps);
-return ret;
+return 0;
 }
 
 
@@ -1789,7 +1774,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr 
conn,
 virObjectEventPtr event = NULL;
 virObjectEventPtr event2 = NULL;
 unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD;
-virCapsPtr caps = NULL;
+VIR_AUTOUNREF(virCapsPtr) caps = NULL;
 unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
VIR_DOMAIN_DEF_PARSE_ABI_UPDATE;
 
@@ -1864,7 +1849,6 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr 
conn,
 virDomainObjEndAPI(&vm);
 virObjectEventStateQueue(driver->domainEventState, event);
 virObjectEventStateQueue(driver->domainEventState, event2);
-virObjectUnref(caps);
 virNWFilterUnlockFilterUpdates();
 return dom;
 }
@@ -3324,7 +3308,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver,
 int ret = -1;
 virObjectEventPtr event = NULL;
 qemuDomainObjPrivatePtr priv = vm->privateData;
-virCapsPtr caps;
+VIR_AUTOUNREF(virCapsPtr) caps = NULL;
 virQEMUSaveDataPtr data = NULL;
 qemuDomainSaveCookiePtr cookie = NULL;
 

[libvirt] [PATCH v2 3/3] qemu_process.c: use GLib macros

[Daniel Henrique Barboza]

Previous patches added VIR_AUTO* facilities in qemu_process.c to
cleanup virObjectUnref() and VIR_FREE() calls. This patch now
converts those VIR_AUTO* macros to their GLib alternatives,
g_autoptr and g_autofree.

The following functions weren't changed in this patch:

- qemuProcessUpdateCPU - error when trying to use g_autoptr with
virDomainCapsCPUModelsPtr;
- qemuProcessLaunch - error when trying to use g_autoptr with
qemuDomainLogContextPtr;
- qemuProcessQMPConnectMonitor - error when trying to use g_autoptr
with virDomainXMLOptionPtr.

The rest of the file was converted to GLib auto* macros.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_process.c | 136 
 1 file changed, 68 insertions(+), 68 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 72919c370d..7cf47207da 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -106,9 +106,9 @@ qemuProcessRemoveDomainStatus(virQEMUDriverPtr driver,
   virDomainObjPtr vm)
 {
 char ebuf[1024];
-VIR_AUTOFREE(char *) file = NULL;
+g_autofree char *file = NULL;
 qemuDomainObjPrivatePtr priv = vm->privateData;
-VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 
 if (virAsprintf(&file, "%s/%s.xml", cfg->stateDir, vm->def->name) < 0)
 return -1;
@@ -403,7 +403,7 @@ qemuProcessHandleReset(qemuMonitorPtr mon G_GNUC_UNUSED,
 virQEMUDriverPtr driver = opaque;
 virObjectEventPtr event;
 qemuDomainObjPrivatePtr priv;
-VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 int ret = -1;
 
 virObjectLock(vm);
@@ -458,7 +458,7 @@ qemuProcessFakeReboot(void *opaque)
 virDomainObjPtr vm = opaque;
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virQEMUDriverPtr driver = priv->driver;
-VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 virDomainRunningReason reason = VIR_DOMAIN_RUNNING_BOOTED;
 int ret = -1, rc;
 
@@ -570,7 +570,7 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon G_GNUC_UNUSED,
 virQEMUDriverPtr driver = opaque;
 qemuDomainObjPrivatePtr priv;
 virObjectEventPtr event = NULL;
-VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 int detail = 0;
 
 VIR_DEBUG("vm=%p", vm);
@@ -641,7 +641,7 @@ qemuProcessHandleStop(qemuMonitorPtr mon G_GNUC_UNUSED,
 virObjectEventPtr event = NULL;
 virDomainPausedReason reason;
 virDomainEventSuspendedDetailType detail;
-VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 qemuDomainObjPrivatePtr priv = vm->privateData;
 
 virObjectLock(vm);
@@ -699,7 +699,7 @@ qemuProcessHandleResume(qemuMonitorPtr mon G_GNUC_UNUSED,
 {
 virQEMUDriverPtr driver = opaque;
 virObjectEventPtr event = NULL;
-VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 qemuDomainObjPrivatePtr priv;
 virDomainRunningReason reason = VIR_DOMAIN_RUNNING_UNPAUSED;
 virDomainEventResumedDetailType eventDetail;
@@ -743,7 +743,7 @@ qemuProcessHandleRTCChange(qemuMonitorPtr mon G_GNUC_UNUSED,
 {
 virQEMUDriverPtr driver = opaque;
 virObjectEventPtr event = NULL;
-VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 
 virObjectLock(vm);
 
@@ -788,7 +788,7 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon G_GNUC_UNUSED,
 virQEMUDriverPtr driver = opaque;
 virObjectEventPtr watchdogEvent = NULL;
 virObjectEventPtr lifecycleEvent = NULL;
-VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 
 virObjectLock(vm);
 watchdogEvent = virDomainEventWatchdogNewFromObj(vm, action);
@@ -856,7 +856,7 @@ qemuProcessHandleIOError(qemuMonitorPtr mon G_GNUC_UNUSED,
 const char *srcPath;
 const char *devAlias;
 virDomainDiskDefPtr disk;
-VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 
 virObjectLock(vm);
 
@@ -923,7 +923,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon G_GNUC_UNUSED,
 virQEMUDriverPtr driver = opaque;
 struct qemuProcessEvent *processEvent = NULL;
 virDomainDiskDefPtr disk;
-VIR_AUTOUNREF(qemuBlockJobDataPtr) job = NULL;
+g_autoptr(qemuBlockJobData) j

[libvirt] [PATCH v2 0/3] qemu_process: use VIR_AUTO*/g_auto* all around

[Daniel Henrique Barboza]

The usual AUTOFREE() and AUTOUNREF() changes that allows
for a bit of cleanup. A new patch was added to convert
most of the VIR_AUTO* macros to GLib g_auto* ones.


v1: https://www.redhat.com/archives/libvir-list/2019-September/msg01465.html 

Daniel Henrique Barboza (3):
  qemu_process: use VIR_AUTOFREE()
  qemu_process: use VIR_AUTOUNREF()
  qemu_process.c: use GLib macros

 src/qemu/qemu_process.c | 444 +++-
 1 file changed, 161 insertions(+), 283 deletions(-)

-- 
2.21.0

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

[libvirt] [PATCH v2 1/3] qemu_process: use VIR_AUTOFREE()

[Daniel Henrique Barboza]

Change all strings and scalar pointers to use VIR_AUTOFREE(),
clearing up the 'cleanup' labels when possible.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_process.c | 118 ++--
 1 file changed, 40 insertions(+), 78 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 9c50c4a1d8..f4b3144154 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -106,7 +106,7 @@ qemuProcessRemoveDomainStatus(virQEMUDriverPtr driver,
   virDomainObjPtr vm)
 {
 char ebuf[1024];
-char *file = NULL;
+VIR_AUTOFREE(char *) file = NULL;
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 int ret = -1;
@@ -117,7 +117,6 @@ qemuProcessRemoveDomainStatus(virQEMUDriverPtr driver,
 if (unlink(file) < 0 && errno != ENOENT && errno != ENOTDIR)
 VIR_WARN("Failed to remove domain XML for %s: %s",
  vm->def->name, virStrerror(errno, ebuf, sizeof(ebuf)));
-VIR_FREE(file);
 
 if (priv->pidfile &&
 unlink(priv->pidfile) < 0 &&
@@ -1513,7 +1512,7 @@ qemuProcessHandleBlockThreshold(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 virDomainDiskDefPtr disk;
 virStorageSourcePtr src;
 unsigned int idx;
-char *dev = NULL;
+VIR_AUTOFREE(char *) dev = NULL;
 const char *path = NULL;
 
 virObjectLock(vm);
@@ -1526,11 +1525,9 @@ qemuProcessHandleBlockThreshold(qemuMonitorPtr mon 
G_GNUC_UNUSED,
 if (virStorageSourceIsLocalStorage(src))
 path = src->path;
 
-if ((dev = qemuDomainDiskBackingStoreGetName(disk, src, idx))) {
+if ((dev = qemuDomainDiskBackingStoreGetName(disk, src, idx)))
 event = virDomainEventBlockThresholdNewFromObj(vm, dev, path,
threshold, excess);
-VIR_FREE(dev);
-}
 }
 
 virObjectUnlock(vm);
@@ -2065,7 +2062,7 @@ static int
 qemuProcessReportLogError(qemuDomainLogContextPtr logCtxt,
   const char *msgprefix)
 {
-char *logmsg = NULL;
+VIR_AUTOFREE(char *) logmsg = NULL;
 size_t max;
 
 max = VIR_ERROR_MAX_LENGTH - 1;
@@ -2082,7 +2079,6 @@ qemuProcessReportLogError(qemuDomainLogContextPtr logCtxt,
 else
 virReportError(VIR_ERR_INTERNAL_ERROR, _("%s: %s"), msgprefix, logmsg);
 
-VIR_FREE(logmsg);
 return 0;
 }
 
@@ -2102,9 +2098,8 @@ qemuProcessLookupPTYs(virDomainChrDefPtr *devices,
   int count,
   virHashTablePtr info)
 {
-char *id = NULL;
+VIR_AUTOFREE(char *) id = NULL;
 size_t i;
-int ret = -1;
 
 for (i = 0; i < count; i++) {
 virDomainChrDefPtr chr = devices[i];
@@ -2123,7 +2118,7 @@ qemuProcessLookupPTYs(virDomainChrDefPtr *devices,
  */
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("no assigned pty for device %s"), id);
-goto cleanup;
+return -1;
 } else {
 /* 'info chardev' had no pty path for this chardev,
  * but the log output had, so we're fine
@@ -2134,14 +2129,11 @@ qemuProcessLookupPTYs(virDomainChrDefPtr *devices,
 
 VIR_FREE(chr->source->data.file.path);
 if (VIR_STRDUP(chr->source->data.file.path, entry->ptyPath) < 0)
-goto cleanup;
+return -1;
 }
 }
 
-ret = 0;
- cleanup:
-VIR_FREE(id);
-return ret;
+return 0;
 }
 
 static int
@@ -2193,8 +2185,7 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriverPtr 
driver,
 int agentReason = VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL;
 qemuMonitorChardevInfoPtr entry;
 virObjectEventPtr event = NULL;
-char *id = NULL;
-int ret = -1;
+VIR_AUTOFREE(char *) id = NULL;
 
 if (booted)
 agentReason = 
VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_DOMAIN_STARTED;
@@ -2205,7 +2196,7 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriverPtr 
driver,
 
 VIR_FREE(id);
 if (virAsprintf(&id, "char%s", chr->info.alias) < 0)
-goto cleanup;
+return -1;
 
 /* port state not reported */
 if (!(entry = virHashLookup(info, id)) ||
@@ -,10 +2213,7 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriverPtr 
driver,
 }
 }
 
-ret = 0;
- cleanup:
-VIR_FREE(id);
-return ret;
+return 0;
 }
 
 
@@ -2645,7 +2633,7 @@ qemuProcessSetupPid(virDomainObjPtr vm,
 virCgroupPtr cgroup = NULL;
 virBitmapPtr use_cpumask = NULL;
 VIR_AUTOPTR(virBitmap) hostcpumap = NULL;
-char *mem_mask = NULL;
+VIR_AUTOFREE(char *) mem_mask = NULL;
 int ret = -1;
 
 if ((period || quota) &&
@@ -2722,7 +2710,6 @@ qemuProcessSetupPid(virDomainObjPtr vm,
 
 ret = 0;
  cleanup:
-V

[libvirt] [PATCH v2 2/3] qemu_process: use VIR_AUTOUNREF()

[Daniel Henrique Barboza]

Change all feasible vir*Ptr pointers to be AUTOUNREF(), clearing
up the 'cleanup' labels when possible.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_process.c | 316 +++-
 1 file changed, 116 insertions(+), 200 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f4b3144154..72919c370d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -108,11 +108,10 @@ qemuProcessRemoveDomainStatus(virQEMUDriverPtr driver,
 char ebuf[1024];
 VIR_AUTOFREE(char *) file = NULL;
 qemuDomainObjPrivatePtr priv = vm->privateData;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-int ret = -1;
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 
 if (virAsprintf(&file, "%s/%s.xml", cfg->stateDir, vm->def->name) < 0)
-goto cleanup;
+return -1;
 
 if (unlink(file) < 0 && errno != ENOENT && errno != ENOTDIR)
 VIR_WARN("Failed to remove domain XML for %s: %s",
@@ -124,10 +123,7 @@ qemuProcessRemoveDomainStatus(virQEMUDriverPtr driver,
 VIR_WARN("Failed to remove PID file for %s: %s",
  vm->def->name, virStrerror(errno, ebuf, sizeof(ebuf)));
 
-ret = 0;
- cleanup:
-virObjectUnref(cfg);
-return ret;
+return 0;
 }
 
 
@@ -407,7 +403,7 @@ qemuProcessHandleReset(qemuMonitorPtr mon G_GNUC_UNUSED,
 virQEMUDriverPtr driver = opaque;
 virObjectEventPtr event;
 qemuDomainObjPrivatePtr priv;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 int ret = -1;
 
 virObjectLock(vm);
@@ -444,7 +440,6 @@ qemuProcessHandleReset(qemuMonitorPtr mon G_GNUC_UNUSED,
  cleanup:
 virObjectUnlock(vm);
 virObjectEventStateQueue(driver->domainEventState, event);
-virObjectUnref(cfg);
 return ret;
 }
 
@@ -463,7 +458,7 @@ qemuProcessFakeReboot(void *opaque)
 virDomainObjPtr vm = opaque;
 qemuDomainObjPrivatePtr priv = vm->privateData;
 virQEMUDriverPtr driver = priv->driver;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 virDomainRunningReason reason = VIR_DOMAIN_RUNNING_BOOTED;
 int ret = -1, rc;
 
@@ -513,7 +508,6 @@ qemuProcessFakeReboot(void *opaque)
 if (ret == -1)
 ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_FORCE));
 virDomainObjEndAPI(&vm);
-virObjectUnref(cfg);
 }
 
 
@@ -576,7 +570,7 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon G_GNUC_UNUSED,
 virQEMUDriverPtr driver = opaque;
 qemuDomainObjPrivatePtr priv;
 virObjectEventPtr event = NULL;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 int detail = 0;
 
 VIR_DEBUG("vm=%p", vm);
@@ -633,7 +627,6 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon G_GNUC_UNUSED,
  unlock:
 virObjectUnlock(vm);
 virObjectEventStateQueue(driver->domainEventState, event);
-virObjectUnref(cfg);
 
 return 0;
 }
@@ -648,7 +641,7 @@ qemuProcessHandleStop(qemuMonitorPtr mon G_GNUC_UNUSED,
 virObjectEventPtr event = NULL;
 virDomainPausedReason reason;
 virDomainEventSuspendedDetailType detail;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 qemuDomainObjPrivatePtr priv = vm->privateData;
 
 virObjectLock(vm);
@@ -694,7 +687,6 @@ qemuProcessHandleStop(qemuMonitorPtr mon G_GNUC_UNUSED,
 
 virObjectUnlock(vm);
 virObjectEventStateQueue(driver->domainEventState, event);
-virObjectUnref(cfg);
 
 return 0;
 }
@@ -707,7 +699,7 @@ qemuProcessHandleResume(qemuMonitorPtr mon G_GNUC_UNUSED,
 {
 virQEMUDriverPtr driver = opaque;
 virObjectEventPtr event = NULL;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 qemuDomainObjPrivatePtr priv;
 virDomainRunningReason reason = VIR_DOMAIN_RUNNING_UNPAUSED;
 virDomainEventResumedDetailType eventDetail;
@@ -740,7 +732,6 @@ qemuProcessHandleResume(qemuMonitorPtr mon G_GNUC_UNUSED,
 
 virObjectUnlock(vm);
 virObjectEventStateQueue(driver->domainEventState, event);
-virObjectUnref(cfg);
 return 0;
 }
 
@@ -752,7 +743,7 @@ qemuProcessHandleRTCChange(qemuMonitorPtr mon G_GNUC_UNUSED,
 {
 virQEMUDriverPtr driver = opaque;
 virObjectEventPtr event = NULL;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
 
 virObjectLock(vm);
 
@@ -784,7 +775,6 @@ qemuProcessHandleRTCChange(qemuMonitorPtr mon G_GNUC_UNUSED,
 virObjectUnlock(vm);
 
 virObjectEventStateQueue(driver->domainEventState, ev

[libvirt] [PATCH v3] util: Block SIGPIPE until execve in child process

[Wang Yechao]

Libvirtd has set SIGPIPE to ignored, and virFork resets all signal
handlers to the defaults. But child process may write logs to
stderr/stdout, that may generate SIGPIPE if journald has stopped.

So block SIGPIPE in virFork, and unblock it before execve.

Signed-off-by: Wang Yechao 
---
v1 patch:
https://www.redhat.com/archives/libvir-list/2019-October/msg00720.html

Changes in v2:
 -  use pthread_sigmask to block SIGPIPE

Changes in v3:
 -  pass SIG_UNBLOCK(not SIG_SETMASK) to pthread_sigmask when unlock SIGPIPE

---
 src/util/vircommand.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 79e1e87..bd3a582 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -321,6 +321,15 @@ virFork(void)
 virDispatchError(NULL);
 _exit(EXIT_CANCELED);
 }
+
+sigemptyset(&newmask);
+sigaddset(&newmask, SIGPIPE);
+if (pthread_sigmask(SIG_BLOCK, &newmask, NULL) != 0) {
+virReportSystemError(errno, "%s", _("cannot block SIGPIPE"));
+virDispatchError(NULL);
+_exit(EXIT_CANCELED);
+}
+
 }
 return pid;
 }
@@ -553,6 +562,7 @@ virExec(virCommandPtr cmd)
 struct sigaction waxon, waxoff;
 VIR_AUTOFREE(gid_t *) groups = NULL;
 int ngroups;
+sigset_t set;
 
 if (cmd->args[0][0] != '/') {
 if (!(binary = binarystr = virFindFileInPath(cmd->args[0]))) {
@@ -792,6 +802,13 @@ virExec(virCommandPtr cmd)
 /* Close logging again to ensure no FDs leak to child */
 virLogReset();
 
+sigemptyset(&set);
+sigaddset(&set, SIGPIPE);
+if (pthread_sigmask(SIG_UNBLOCK, &set, NULL) != 0) {
+virReportSystemError(errno, "%s", _("cannot unblock SIGPIPE"));
+goto fork_error;
+}
+
 if (cmd->env)
 execve(binary, cmd->args, cmd->env);
 else
-- 
1.8.3.1

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

Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support

[Han Han]

On Wed, Oct 16, 2019 at 1:04 AM Cole Robinson  wrote:

> On 10/15/19 3:56 AM, Han Han wrote:
> > Hello Cole, one issue is found:
> > The qcow2 data file XTTRs is not cleaned on external snapshot when
> > -blockdev is not enabled
> >
> > Versions:
> > libvirt v5.8.0-134-g9d03e9adf1
> > qemu-kvm-4.1.0-13.module+el8.1.0+4313+ef76ec61.x86_64
> >
> > Steps:
> > 1. Convert a OS image to qcow2&qcow2 data file:
> > # qemu-img convert -O qcow2 -o
> > data_file=/var/lib/libvirt/images/pc-data.raw,data_file_raw=on
> > /var/lib/libvirt/images/pc.qcow2 /var/lib/libvirt/images/pc-data.qcow2
> >
> > 2. Build and start libvirt source, start libvirt daemon:
> > # make clean && CC=/usr/lib64/ccache/cc ./autogen.sh&&./configure
> > --without-libssh --build=x86_64-redhat-linux-gnu
> > --host=x86_64-redhat-linux-gnu --program-prefix=
> > --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
> > --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
> > --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
> > --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib
> > --mandir=/usr/share/man --infodir=/usr/share/info --with-qemu
> > --without-openvz --without-lxc --without-vbox --without-libxl
> > --with-sasl --with-polkit --with-libvirtd --without-phyp --with-esx
> > --without-hyperv --without-vmware --without-xenapi --without-vz
> > --without-bhyve --with-interface --with-network --with-storage-fs
> > --with-storage-lvm --with-storage-iscsi --with-storage-iscsi-direct
> > --with-storage-scsi --with-storage-disk --with-storage-mpath
> > --with-storage-rbd --without-storage-sheepdog --with-storage-gluster
> > --without-storage-zfs --without-storage-vstorage --with-numactl
> > --with-numad --with-capng --without-fuse --with-netcf --with-selinux
> > --with-selinux-mount=/sys/fs/selinux --without-apparmor --without-hal
> > --with-udev --with-yajl --with-sanlock --with-libpcap --with-macvtap
> > --with-audit --with-dtrace --with-driver-modules --with-firewalld
> > --with-firewalld-zone --without-wireshark-dissector --without-pm-utils
> > --with-nss-plugin '--with-packager=Unknown, 2019-08-19-12:13:01,
> > lab.rhel8.me ' --with-packager-version=1.el8
> > --with-qemu-user=qemu --with-qemu-group=qemu
> > --with-tls-priority=@LIBVIRT,SYSTEM --enable-werror
> > --enable-expensive-tests --with-init-script=systemd
> > --without-login-shell && make -j8
> > # LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" src/.libs/virtlogd
> > # LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" LIBVIRT_DEBUG=3
> > LIBVIRT_LOG_FILTERS="1:util 1:qemu 1:security"
> > LIBVIRT_LOG_OUTPUTS="1:file:/tmp/libvirt_daemon.log" src/.libs/libvirtd
> >
> > 3. Define and start an VM with the qcow2&qcow2 data file. Note that the
> > -blockdev is not enabled
> > # virsh define pc-data.xml
> > # virsh start pc-data
> >
> > 4. Create snapshot and check the data file XATTRs:
> > # virsh snapshot-create-as pc-data s1 --no-metadata --disk-only
> > # getfattr -m - -d /var/lib/libvirt/images/pc-data.raw
> > getfattr: Removing leading '/' from absolute path names
> > # file: var/lib/libvirt/images/pc-data.raw
> > security.selinux="unconfined_u:object_r:svirt_image_t:s0:c775,c1011"
> > trusted.libvirt.security.dac="+107:+107"
> > trusted.libvirt.security.ref_dac="1"
> > trusted.libvirt.security.ref_selinux="1"
> >
> trusted.libvirt.security.selinux="unconfined_u:object_r:svirt_image_t:s0:c284,c367"
> > trusted.libvirt.security.timestamp_dac="1563328069"
> > trusted.libvirt.security.timestamp_selinux="1563328069"
> >
> > Shutdown the VM. The XATTRs of data file is not changed.
> > It is not expected. The XTTRs should not contain *.libvirt.*
> >
> > Issue is not reproduced with -blockdev enabled:
> > 
> > ...
> >   
> > 
> > 
> >   
> > 
> >
> > See the libvirt daemon log and vm xml in attachment.
>
> Nice catch! I will need to dig into this to figure out where the issue
> is. Can you put this info into an upstream bug report in
>
Sure. https://bugzilla.redhat.com/show_bug.cgi?id=1762135

> product=Virtualization Tools  and I will get to it when I can
>
> Thanks,
> Cole
>


-- 
Best regards,
---
Han Han
Quality Engineer
Redhat.

Email: h...@redhat.com
Phone: +861065339333
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3] util: Block SIGPIPE until execve in child process

[Eric Blake]

On 10/15/19 9:25 PM, Wang Yechao wrote:

Libvirtd has set SIGPIPE to ignored, and virFork resets all signal
handlers to the defaults. But child process may write logs to
stderr/stdout, that may generate SIGPIPE if journald has stopped.

So block SIGPIPE in virFork, and unblock it before execve.


How does that help?  If writing to stderr hits EOF, it will queue up a 
SIGPIPE to be delivered as soon as you unblock SIGPIPE.


The correct fix is to not unblock SIGPIPE until after any point at which 
you would be performing I/O that must not fail due to SIGPIPE, rather 
than messing with signal masks to just delay when SIGPIPE is delivered.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

[libvirt] [PATCH 1/3] conf/domain_conf: use virStringParseYesNo helper

[Mao Zhongyi]

This helper performs a conversion from a "yes|no" string
to a corresponding boolean, and several conversions were
already done, but there are still some omissions.

For most of the remaining usages in domain_conf.c only
"yes" is explicitly checked for. This means all other
values are implicitly handled as 'false'. In this case,
use virStringParseYesNo, but if it returns -1, don't
raise an error but set the bool value to false.

Cc: crobi...@redhat.com
Cc: berra...@redhat.com
Cc: abolo...@redhat.com
Cc: la...@laine.org
Cc: phrd...@redhat.com
Cc: mklet...@redhat.com
Cc: g.sho1...@gmail.com

Signed-off-by: Mao Zhongyi 
Signed-off-by: Zhang Shengju 
---
 src/conf/domain_conf.c | 30 ++
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 10d6bf0eea..7420658726 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7601,8 +7601,8 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
 }
 
 if ((autoAddress = virXMLPropString(node, "autoAddress"))) {
-if (STREQ(autoAddress, "yes"))
-usbsrc->autoAddress = true;
+if (virStringParseYesNo(autoAddress, &usbsrc->autoAddress) < 0)
+usbsrc->autoAddress = false;
 }
 
 /* Product can validly be 0, so we need some extra help to determine
@@ -8168,8 +8168,8 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
  * (e.g. ) with type='hostdev')
  */
 if ((managed = virXMLPropString(node, "managed")) != NULL) {
-if (STREQ(managed, "yes"))
-def->managed = true;
+if (virStringParseYesNo(managed, &def->managed) < 0)
+def->managed = false;
 }
 
 sgio = virXMLPropString(node, "sgio");
@@ -13807,9 +13807,7 @@ 
virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def,
 
 if (autoGenerated &&
 flags & VIR_DOMAIN_DEF_PARSE_STATUS) {
-if (STREQ(autoGenerated, "yes")) {
-def->autoGenerated = true;
-} else if (STRNEQ(autoGenerated, "no")) {
+if (virStringParseYesNo(autoGenerated, &def->autoGenerated) < 0) {
 virReportError(VIR_ERR_XML_ERROR,
_("Invalid autoGenerated value: %s"),
autoGenerated);
@@ -13939,12 +13937,11 @@ 
virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr def,
 }
 
 if (autoport) {
-if (STREQ(autoport, "yes")) {
-if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)
-def->data.vnc.port = 0;
-def->data.vnc.autoport = true;
-} else {
+if (virStringParseYesNo(autoport, &def->data.vnc.autoport) < 0) {
 def->data.vnc.autoport = false;
+} else {
+if (def->data.vnc.autoport && flags & 
VIR_DOMAIN_DEF_PARSE_INACTIVE)
+def->data.vnc.port = 0;
 }
 }
 
@@ -13958,8 +13955,9 @@ virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr 
def,
 }
 }
 
-if (websocketGenerated && STREQ(websocketGenerated, "yes"))
-def->data.vnc.websocketGenerated = true;
+if (websocketGenerated &&
+virStringParseYesNo(websocketGenerated, 
&def->data.vnc.websocketGenerated))
+def->data.vnc.websocketGenerated = false;
 
 if (sharePolicy) {
 int policy =
@@ -15479,8 +15477,8 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
 heads = virXMLPropString(cur, "heads");
 
 if ((primary = virXMLPropString(cur, "primary")) != NULL) {
-if (STREQ(primary, "yes"))
-def->primary = true;
+if (virStringParseYesNo(primary, &def->primary) < 0)
+def->primary = false;
 VIR_FREE(primary);
 }
 
-- 
2.17.1



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

[libvirt] [PATCH 3/3] qemu/qemu_migration_params: use virStringParseYesNo helper

[Mao Zhongyi]

A function virStringParseYesNo was added to convert
string 'yes' to true and 'no' to false, so use this
helper to replace 'STREQ(.*, \"yes\")' and
'STREQ(.*, \"no\")' as it allows us to drop several
repetitive if-then-else string->bool conversion blocks.

Cc: jdene...@redhat.com
Cc: g.sho1...@gmail.com
Cc: crobi...@redhat.com

Signed-off-by: Mao Zhongyi 
Signed-off-by: Zhang Shengju 
---
 src/qemu/qemu_migration_params.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
index 85fa8f8de5..acd42042fe 100644
--- a/src/qemu/qemu_migration_params.c
+++ b/src/qemu/qemu_migration_params.c
@@ -1326,11 +1326,7 @@ qemuMigrationParamsParse(xmlXPathContextPtr ctxt,
 break;
 
 case QEMU_MIGRATION_PARAM_TYPE_BOOL:
-if (STREQ(value, "yes"))
-pv->value.b = true;
-else if (STREQ(value, "no"))
-pv->value.b = false;
-else
+if (virStringParseYesNo(value, &pv->value.b) < 0)
 rc = -1;
 break;
 
-- 
2.17.1



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

[libvirt] [PATCH 0/3] use virStringParseYesNo helper

[Mao Zhongyi]

A function virStringParseYesNo was added to convert
string 'yes' to true and 'no' to false, so use this
helper to replace 'STREQ(.*, \"yes\")' and
'STREQ(.*, \"no\")' as it allows us to drop several
repetitive if-then-else string->bool conversion blocks.

Cc: abolo...@redhat.com
Cc: berra...@redhat.com
Cc: crobi...@redhat.com
Cc: g...@czarc.net
Cc: g.sho1...@gmail.com
Cc: jdene...@redhat.com
Cc: la...@laine.org
Cc: mklet...@redhat.com
Cc: phrd...@redhat.com

Mao Zhongyi (3):
  conf/domain_conf: use virStringParseYesNo helper
  conf/network_conf: use virStringParseYesNo helper
  qemu/qemu_migration_params: use virStringParseYesNo helper

 src/conf/domain_conf.c   | 30 ++
 src/conf/network_conf.c  |  4 +---
 src/qemu/qemu_migration_params.c |  6 +-
 3 files changed, 16 insertions(+), 24 deletions(-)

-- 
2.17.1



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

[libvirt] [PATCH 2/3] conf/network_conf: use virStringParseYesNo helper

[Mao Zhongyi]

A function virStringParseYesNo was added to convert
string 'yes' to true and 'no' to false, so use this
helper to replace 'STREQ(.*, \"yes\")' and
'STREQ(.*, \"no\")' as it allows us to drop several
repetitive if-then-else string->bool conversion blocks.

Cc: g...@czarc.net
Cc: crobi...@redhat.com
Cc: berra...@redhat.com
Cc: g.sho1...@gmail.com

Signed-off-by: Mao Zhongyi 
Signed-off-by: Zhang Shengju 
---
 src/conf/network_conf.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 75ec5ccc27..9954c3d25f 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1682,9 +1682,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt,
  */
 ipv6nogwStr = virXPathString("string(./@ipv6)", ctxt);
 if (ipv6nogwStr) {
-if (STREQ(ipv6nogwStr, "yes")) {
-def->ipv6nogw = true;
-} else if (STRNEQ(ipv6nogwStr, "no")) {
+if (virStringParseYesNo(ipv6nogwStr, &def->ipv6nogw) < 0) {
 virReportError(VIR_ERR_XML_ERROR,
_("Invalid ipv6 setting '%s' in network '%s'"),
ipv6nogwStr, def->name);
-- 
2.17.1



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

Re: [libvirt] [PATCH v3] util: Block SIGPIPE until execve in child process

[Eric Blake]

On 10/15/19 9:43 PM, Eric Blake wrote:

On 10/15/19 9:25 PM, Wang Yechao wrote:

Libvirtd has set SIGPIPE to ignored, and virFork resets all signal
handlers to the defaults. But child process may write logs to
stderr/stdout, that may generate SIGPIPE if journald has stopped.

So block SIGPIPE in virFork, and unblock it before execve.


How does that help?  If writing to stderr hits EOF, it will queue up a 
SIGPIPE to be delivered as soon as you unblock SIGPIPE.


The correct fix is to not unblock SIGPIPE until after any point at which 
you would be performing I/O that must not fail due to SIGPIPE, rather 
than messing with signal masks to just delay when SIGPIPE is delivered.


Let's word this better:

The correct fix is to keep SIGPIPE ignored until after any point at 
which you would be performing I/O that must not cause SIGPIPE, reverting 
SIGPIPE back to SIG_DFL at the last possible moment.  Messing with 
signal masks merely delays when SIGPIPE is delivered, rather than 
avoiding it.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

Re: [libvirt] [PATCH 1/1] Add shrink flag to blockresize command

[Han Han]

On Tue, Oct 15, 2019 at 10:46 PM Cole Robinson  wrote:

> On 10/5/19 2:56 PM, martinsson.pat...@gmail.com wrote:
> > From: patchon 
> >
>
> You probably want to fix this to match your signed-off-by name.
>
> > These commits simply adds the '--shrink' flag to the blockresize
>
I agree that an options should be added here to avoid undesirable disk
shrinking.
Because I have broken guest OS image for several times due to the shrinking
of
blockresize.

> > command to prevent accidental shrinking of a block device. This
> > behaviour is already present on the vol-resize command and it
> > makes sense to mimic that behaviour.
> >
>
> I don't have an opinion on whether the feature should be added at the
> API level, I will leave that to others. CCing pkrempa
>
> But as implemented it seems problematic to add a flag that changes the
> semantics of the API, essentially requiring a new flag to get the old
> behavior. I see the argument that this is a data safety issue, but maybe
> apps are depending on this already? I don't know enough about the usage
> to guess either way
>
> I disagree the check of disk shrink is added to API level because it
changes
the behavior of blockresize API and may cause unexpected error in uplayer
products.

It is better to add the check to codes of **virsh** to avoid undesirable
shrinking
in cmdline user. And we can use '--force' to make blockresize always.
The procedure will be:

if (new size < capacity && no --force)
print error("Can't shrink capacity below current unless --force
specified")
else
call blockresize

And also we can add comments to blockresize API to avoid uplayer use
undesirable disk shrinking.

> > Only implemented in the qemu-driver atm.
> >
> > Signed-off-by: Patrik Martinsson 
> > ---
> >  src/qemu/qemu_driver.c | 15 ++-
> >  tools/virsh-domain.c   |  7 +++
> >  tools/virsh.pod|  9 -
> >  3 files changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 95fe844c34..4e34eec796 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -11265,8 +11265,10 @@ qemuDomainBlockResize(virDomainPtr dom,
> >  char *device = NULL;
> >  const char *nodename = NULL;
> >  virDomainDiskDefPtr disk = NULL;
> > +virDomainBlockInfo info;
> >
> > -virCheckFlags(VIR_DOMAIN_BLOCK_RESIZE_BYTES, -1);
> > +virCheckFlags(VIR_DOMAIN_BLOCK_RESIZE_BYTES |
> > +  VIR_STORAGE_VOL_RESIZE_SHRINK, -1);
> >
>
> We shouldn't mix and match flag prefix names. Each public API should be
> coupled with its own set of flag names. So you would want to call this
> VIR_DOMAIN_BLOCK_RESIZE_SHRINK, and add it to
> include/libvirt/libvirt-domain.h and add it to documentation in
> libvirt-domain.c
>
> - Cole
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>


-- 
Best regards,
---
Han Han
Quality Engineer
Redhat.

Email: h...@redhat.com
Phone: +861065339333
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list