[PATCH 00/20] qemu: migration_cookie: Refactor and modernize
Note that this applies on top of the recent bitmap refactors Peter Krempa (20): qemu: migration_cookie: Extract parsing/validation of mandatory features qemuMigrationCookieXMLParse: Switch to single-purpose temporary variables qemuMigrationCookieXMLParse: Check domain element count more defensively qemuMigrationCookieXMLParse: Decrease scope of 'nodes' and use automatic freeing qemuMigrationCookieXMLParse: Remove comment mentioning that error was already set qemuMigrationCookieXMLParse: Remove 'error' label qemuMigrationCookieGraphicsXMLFormat: Use 'virXMLFormatElement' qemuMigrationCookieNetworkXMLFormat: Refactor XML formatting qemuMigrationCookieXMLFormat: Extract formatting of NBD qemuDomainExtractTLSSubject: Refactor memory handling qemu: migration_cookie: Register 'autoptr' functions for internal types qemuMigrationCookieGraphicsSpiceAlloc: Refactor memory handling qemuMigrationCookieNetworkAlloc: Refactor memory handling qemuMigrationCookieXMLFormat: Refactor memory handling qemuMigrationCookieNetworkXMLParse: Refactor memory handling qemuMigrationCookieNBDXMLParse: Refactor memory handling qemuMigrationCookieCapsXMLParse: Refactor memory handling qemuMigrationCookieAddCaps: Use 'g_new0' instead of VIR_ALLOC qemuMigrationCookieXMLParse: Avoid VIR_FREE when parsing lockstate qemu: migration_cookie: s/VIR_FREE/g_free/ src/qemu/qemu_migration_cookie.c | 493 ++++++++++++++----------------- 1 file changed, 214 insertions(+), 279 deletions(-) -- 2.26.2
Move the code into 'qemuMigrationCookieXMLParseMandatoryFeatures' to simplify 'qemuMigrationCookieXMLParse'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 77 +++++++++++++++++++------------- 1 file changed, 45 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 4b73a98f7e..22705874e0 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -1159,6 +1159,49 @@ qemuMigrationCookieCapsXMLParse(xmlXPathContextPtr ctxt) } +/** + * qemuMigrationCookieXMLParseMandatoryFeatures: + * + * Check to ensure all mandatory features from XML are also present in 'flags'. + */ +static int +qemuMigrationCookieXMLParseMandatoryFeatures(xmlXPathContextPtr ctxt, + unsigned int flags) +{ + g_autofree xmlNodePtr *nodes = NULL; + size_t i; + ssize_t n; + + if ((n = virXPathNodeSet("./feature", ctxt, &nodes)) < 0) + return -1; + + for (i = 0; i < n; i++) { + int val; + g_autofree char *str = virXMLPropString(nodes[i], "name"); + + if (!str) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("missing feature name")); + return -1; + } + + if ((val = qemuMigrationCookieFlagTypeFromString(str)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown migration cookie feature %s"), str); + return -1; + } + + if ((flags & (1 << val)) == 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unsupported migration cookie feature %s"), str); + return -1; + } + } + + return 0; +} + + static int qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, virQEMUDriverPtr driver, @@ -1170,7 +1213,6 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, char uuidstr[VIR_UUID_STRING_BUFLEN]; char *tmp = NULL; xmlNodePtr *nodes = NULL; - size_t i; int n; /* We don't store the uuid, name, hostname, or hostuuid @@ -1237,38 +1279,9 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, } VIR_FREE(tmp); - /* Check to ensure all mandatory features from XML are also - * present in 'flags' */ - if ((n = virXPathNodeSet("./feature", ctxt, &nodes)) < 0) - goto error; - - for (i = 0; i < n; i++) { - int val; - char *str = virXMLPropString(nodes[i], "name"); - if (!str) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing feature name")); - goto error; - } - - if ((val = qemuMigrationCookieFlagTypeFromString(str)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown migration cookie feature %s"), - str); - VIR_FREE(str); - goto error; - } - if ((flags & (1 << val)) == 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unsupported migration cookie feature %s"), - str); - VIR_FREE(str); - goto error; - } - VIR_FREE(str); - } - VIR_FREE(nodes); + if (qemuMigrationCookieXMLParseMandatoryFeatures(ctxt, flags) < 0) + return -1; if ((flags & QEMU_MIGRATION_COOKIE_GRAPHICS) && virXPathBoolean("count(./graphics) > 0", ctxt) && -- 2.26.2
Don't reuse 'tmp' over and over, but switch to single use automaticaly freed variables instead. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 22705874e0..f313b6d00e 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -1210,8 +1210,10 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, xmlXPathContextPtr ctxt, unsigned int flags) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - char *tmp = NULL; + g_autofree char *name = NULL; + g_autofree char *uuid = NULL; + g_autofree char *hostuuid = NULL; + char localdomuuid[VIR_UUID_STRING_BUFLEN]; xmlNodePtr *nodes = NULL; int n; @@ -1221,34 +1223,31 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, */ /* Extract domain name */ - if (!(tmp = virXPathString("string(./name[1])", ctxt))) { + if (!(name = virXPathString("string(./name[1])", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing name element in migration data")); goto error; } - if (STRNEQ(tmp, mig->name)) { + if (STRNEQ(name, mig->name)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Incoming cookie data had unexpected name %s vs %s"), - tmp, mig->name); + name, mig->name); goto error; } - VIR_FREE(tmp); /* Extract domain uuid */ - tmp = virXPathString("string(./uuid[1])", ctxt); - if (!tmp) { + if (!(uuid = virXPathString("string(./uuid[1])", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing uuid element in migration data")); goto error; } - virUUIDFormat(mig->uuid, uuidstr); - if (STRNEQ(tmp, uuidstr)) { + virUUIDFormat(mig->uuid, localdomuuid); + if (STRNEQ(uuid, localdomuuid)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Incoming cookie data had unexpected UUID %s vs %s"), - tmp, uuidstr); + uuid, localdomuuid); goto error; } - VIR_FREE(tmp); if (!(mig->remoteHostname = virXPathString("string(./hostname[1])", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1261,12 +1260,12 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, * for sure. */ /* Check & forbid localhost migration */ - if (!(tmp = virXPathString("string(./hostuuid[1])", ctxt))) { + if (!(hostuuid = virXPathString("string(./hostuuid[1])", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing hostuuid element in migration data")); goto error; } - if (virUUIDParse(tmp, mig->remoteHostuuid) < 0) { + if (virUUIDParse(hostuuid, mig->remoteHostuuid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed hostuuid element in migration data")); goto error; @@ -1274,11 +1273,9 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, if (memcmp(mig->remoteHostuuid, mig->localHostuuid, VIR_UUID_BUFLEN) == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Attempt to migrate guest to the same host %s"), - tmp); + hostuuid); goto error; } - VIR_FREE(tmp); - if (qemuMigrationCookieXMLParseMandatoryFeatures(ctxt, flags) < 0) return -1; @@ -1353,7 +1350,6 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, return 0; error: - VIR_FREE(tmp); VIR_FREE(nodes); return -1; } -- 2.26.2
Make sure that 'virXPathNodeSet' returns '1' as the only expected value rather than relying on the fact that the previous check for the number of elements ensures success of the subsequent call. The error message no longer mentions the number of <domain> elements in the cookie, but this is a very unlikely internal error anyways. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index f313b6d00e..9b1268833a 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -1215,7 +1215,6 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, g_autofree char *hostuuid = NULL; char localdomuuid[VIR_UUID_STRING_BUFLEN]; xmlNodePtr *nodes = NULL; - int n; /* We don't store the uuid, name, hostname, or hostuuid * values. We just compare them to local data to do some @@ -1300,11 +1299,9 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, if ((flags & QEMU_MIGRATION_COOKIE_PERSISTENT) && virXPathBoolean("count(./domain) > 0", ctxt)) { - if ((n = virXPathNodeSet("./domain", ctxt, &nodes)) > 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Too many domain elements in " - "migration cookie: %d"), - n); + if ((virXPathNodeSet("./domain", ctxt, &nodes)) != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Too many domain elements in migration cookie")); goto error; } mig->persistent = virDomainDefParseNode(doc, nodes[0], -- 2.26.2
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 9b1268833a..a9dd2b9323 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -1214,7 +1214,6 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, g_autofree char *uuid = NULL; g_autofree char *hostuuid = NULL; char localdomuuid[VIR_UUID_STRING_BUFLEN]; - xmlNodePtr *nodes = NULL; /* We don't store the uuid, name, hostname, or hostuuid * values. We just compare them to local data to do some @@ -1299,6 +1298,8 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, if ((flags & QEMU_MIGRATION_COOKIE_PERSISTENT) && virXPathBoolean("count(./domain) > 0", ctxt)) { + g_autofree xmlNodePtr *nodes = NULL; + if ((virXPathNodeSet("./domain", ctxt, &nodes)) != 1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Too many domain elements in migration cookie")); @@ -1314,7 +1315,6 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, * an error for us */ goto error; } - VIR_FREE(nodes); } if ((flags & QEMU_MIGRATION_COOKIE_NETWORK) && @@ -1347,7 +1347,6 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, return 0; error: - VIR_FREE(nodes); return -1; } -- 2.26.2
Most of our functions report errors so there's no need to mention it here again. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index a9dd2b9323..f7625a7991 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -1310,11 +1310,8 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_ABI_UPDATE_MIGRATION | VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); - if (!mig->persistent) { - /* virDomainDefParseNode already reported - * an error for us */ + if (!mig->persistent) goto error; - } } if ((flags & QEMU_MIGRATION_COOKIE_NETWORK) && -- 2.26.2
Now it only returns -1 so we can do that directly. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 39 +++++++++++++++----------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index f7625a7991..052d8e311b 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -1224,33 +1224,33 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, if (!(name = virXPathString("string(./name[1])", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing name element in migration data")); - goto error; + return -1; } if (STRNEQ(name, mig->name)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Incoming cookie data had unexpected name %s vs %s"), name, mig->name); - goto error; + return -1; } /* Extract domain uuid */ if (!(uuid = virXPathString("string(./uuid[1])", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing uuid element in migration data")); - goto error; + return -1; } virUUIDFormat(mig->uuid, localdomuuid); if (STRNEQ(uuid, localdomuuid)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Incoming cookie data had unexpected UUID %s vs %s"), uuid, localdomuuid); - goto error; + return -1; } if (!(mig->remoteHostname = virXPathString("string(./hostname[1])", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing hostname element in migration data")); - goto error; + return -1; } /* Historically, this is the place where we checked whether remoteHostname * and localHostname are the same. But even if they were, it doesn't mean @@ -1261,18 +1261,18 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, if (!(hostuuid = virXPathString("string(./hostuuid[1])", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing hostuuid element in migration data")); - goto error; + return -1; } if (virUUIDParse(hostuuid, mig->remoteHostuuid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed hostuuid element in migration data")); - goto error; + return -1; } if (memcmp(mig->remoteHostuuid, mig->localHostuuid, VIR_UUID_BUFLEN) == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Attempt to migrate guest to the same host %s"), hostuuid); - goto error; + return -1; } if (qemuMigrationCookieXMLParseMandatoryFeatures(ctxt, flags) < 0) @@ -1281,7 +1281,7 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, if ((flags & QEMU_MIGRATION_COOKIE_GRAPHICS) && virXPathBoolean("count(./graphics) > 0", ctxt) && (!(mig->graphics = qemuMigrationCookieGraphicsXMLParse(ctxt)))) - goto error; + return -1; if ((flags & QEMU_MIGRATION_COOKIE_LOCKSTATE) && virXPathBoolean("count(./lockstate) > 0", ctxt)) { @@ -1289,7 +1289,7 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, if (!mig->lockDriver) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing lock driver name in migration cookie")); - goto error; + return -1; } mig->lockState = virXPathString("string(./lockstate[1]/leases[1])", ctxt); if (mig->lockState && STREQ(mig->lockState, "")) @@ -1303,7 +1303,7 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, if ((virXPathNodeSet("./domain", ctxt, &nodes)) != 1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Too many domain elements in migration cookie")); - goto error; + return -1; } mig->persistent = virDomainDefParseNode(doc, nodes[0], driver->xmlopt, qemuCaps, @@ -1311,40 +1311,37 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE_MIGRATION | VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); if (!mig->persistent) - goto error; + return -1; } if ((flags & QEMU_MIGRATION_COOKIE_NETWORK) && virXPathBoolean("count(./network) > 0", ctxt) && (!(mig->network = qemuMigrationCookieNetworkXMLParse(ctxt)))) - goto error; + return -1; if (flags & QEMU_MIGRATION_COOKIE_NBD && virXPathBoolean("boolean(./nbd)", ctxt) && (!(mig->nbd = qemuMigrationCookieNBDXMLParse(ctxt)))) - goto error; + return -1; if (flags & QEMU_MIGRATION_COOKIE_STATS && virXPathBoolean("boolean(./statistics)", ctxt) && (!(mig->jobInfo = qemuMigrationCookieStatisticsXMLParse(ctxt)))) - goto error; + return -1; if (flags & QEMU_MIGRATION_COOKIE_CPU && virCPUDefParseXML(ctxt, "./cpu[1]", VIR_CPU_TYPE_GUEST, &mig->cpu) < 0) - goto error; + return -1; if (flags & QEMU_MIGRATION_COOKIE_ALLOW_REBOOT && qemuDomainObjPrivateXMLParseAllowReboot(ctxt, &mig->allowReboot) < 0) - goto error; + return -1; if (flags & QEMU_MIGRATION_COOKIE_CAPS && !(mig->caps = qemuMigrationCookieCapsXMLParse(ctxt))) - goto error; + return -1; return 0; - - error: - return -1; } -- 2.26.2
Switch to the two buffer approach to simplify the logic for terminating the element. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 052d8e311b..accf3c5efb 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -581,20 +581,19 @@ static void qemuMigrationCookieGraphicsXMLFormat(virBufferPtr buf, qemuMigrationCookieGraphicsPtr grap) { - virBufferAsprintf(buf, "<graphics type='%s' port='%d' listen='%s'", + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + + virBufferAsprintf(&attrBuf, " type='%s' port='%d' listen='%s'", virDomainGraphicsTypeToString(grap->type), grap->port, grap->listen); + if (grap->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) - virBufferAsprintf(buf, " tlsPort='%d'", grap->tlsPort); - if (grap->tlsSubject) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - virBufferEscapeString(buf, "<cert info='subject' value='%s'/>\n", grap->tlsSubject); - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</graphics>\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } + virBufferAsprintf(&attrBuf, " tlsPort='%d'", grap->tlsPort); + + virBufferEscapeString(&childBuf, "<cert info='subject' value='%s'/>\n", grap->tlsSubject); + + virXMLFormatElement(buf, "graphics", &attrBuf, &childBuf); } -- 2.26.2
Use 'virXMLFormatElement' both for formating the whole <network> element but also for formatting the <interface> subelements. This alows to remove the crazy logic which was determining which element was already formatted. Additional simplification is achieved by switching to skipping the loop using 'continue' rather than putting everything in a giant block. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 40 +++++++++++++------------------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index accf3c5efb..a888c0074c 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -601,35 +601,27 @@ static void qemuMigrationCookieNetworkXMLFormat(virBufferPtr buf, qemuMigrationCookieNetworkPtr optr) { + g_auto(virBuffer) interfaceBuf = VIR_BUFFER_INIT_CHILD(buf); size_t i; - bool empty = true; for (i = 0; i < optr->nnets; i++) { + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(&interfaceBuf); + /* If optr->net[i].vporttype is not set, there is nothing to transfer */ - if (optr->net[i].vporttype != VIR_NETDEV_VPORT_PROFILE_NONE) { - if (empty) { - virBufferAddLit(buf, "<network>\n"); - virBufferAdjustIndent(buf, 2); - empty = false; - } - virBufferAsprintf(buf, "<interface index='%zu' vporttype='%s'", - i, virNetDevVPortTypeToString(optr->net[i].vporttype)); - if (optr->net[i].portdata) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - virBufferEscapeString(buf, "<portdata>%s</portdata>\n", - optr->net[i].portdata); - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</interface>\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } - } - } - if (!empty) { - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</network>\n"); + if (optr->net[i].vporttype == VIR_NETDEV_VPORT_PROFILE_NONE) + continue; + + virBufferAsprintf(&attrBuf, " index='%zu' vporttype='%s'", + i, virNetDevVPortTypeToString(optr->net[i].vporttype)); + + virBufferEscapeString(&childBuf, "<portdata>%s</portdata>\n", + optr->net[i].portdata); + + virXMLFormatElement(&interfaceBuf, "interface", &attrBuf, &childBuf); } + + virXMLFormatElement(buf, "network", NULL, &interfaceBuf); } -- 2.26.2
Move the code into 'qemuMigrationCookieNBDXMLFormat' and use modern XML formatting code patterns. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 41 +++++++++++++++++--------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index a888c0074c..b7d8ba58bf 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -758,6 +758,26 @@ qemuMigrationCookieCapsXMLFormat(virBufferPtr buf, } +static void +qemuMigrationCookieNBDXMLFormat(qemuMigrationCookieNBDPtr nbd, + virBufferPtr buf) +{ + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + size_t i; + + if (nbd->port) + virBufferAsprintf(buf, " port='%d'", nbd->port); + + for (i = 0; i < nbd->ndisks; i++) { + virBufferEscapeString(&childBuf, "<disk target='%s'", nbd->disks[i].target); + virBufferAsprintf(&childBuf, " capacity='%llu'/>\n", nbd->disks[i].capacity); + } + + virXMLFormatElement(buf, "nbd", &attrBuf, &childBuf); +} + + static int qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver, virQEMUCapsPtr qemuCaps, @@ -814,25 +834,8 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver, if ((mig->flags & QEMU_MIGRATION_COOKIE_NETWORK) && mig->network) qemuMigrationCookieNetworkXMLFormat(buf, mig->network); - if ((mig->flags & QEMU_MIGRATION_COOKIE_NBD) && mig->nbd) { - virBufferAddLit(buf, "<nbd"); - if (mig->nbd->port) - virBufferAsprintf(buf, " port='%d'", mig->nbd->port); - if (mig->nbd->ndisks) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - for (i = 0; i < mig->nbd->ndisks; i++) { - virBufferEscapeString(buf, "<disk target='%s'", - mig->nbd->disks[i].target); - virBufferAsprintf(buf, " capacity='%llu'/>\n", - mig->nbd->disks[i].capacity); - } - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</nbd>\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } - } + if ((mig->flags & QEMU_MIGRATION_COOKIE_NBD) && mig->nbd) + qemuMigrationCookieNBDXMLFormat(mig->nbd, buf); if (mig->flags & QEMU_MIGRATION_COOKIE_STATS && mig->jobInfo) qemuMigrationCookieStatisticsXMLFormat(buf, mig->jobInfo); -- 2.26.2
Switch to automatic memory cleaning, use g_new0 for allocation and get rid of the 'error' label. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index b7d8ba58bf..006869a618 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -133,9 +133,9 @@ qemuMigrationCookieFree(qemuMigrationCookiePtr mig) static char * qemuDomainExtractTLSSubject(const char *certdir) { - char *certfile = NULL; + g_autofree char *certfile = NULL; char *subject = NULL; - char *pemdata = NULL; + g_autofree char *pemdata = NULL; gnutls_datum_t pemdatum; gnutls_x509_crt_t cert; int ret; @@ -146,7 +146,7 @@ qemuDomainExtractTLSSubject(const char *certdir) if (virFileReadAll(certfile, 8192, &pemdata) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unable to read server cert %s"), certfile); - goto error; + return NULL; } ret = gnutls_x509_crt_init(&cert); @@ -154,7 +154,7 @@ qemuDomainExtractTLSSubject(const char *certdir) virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot initialize cert object: %s"), gnutls_strerror(ret)); - goto error; + return NULL; } pemdatum.data = (unsigned char *)pemdata; @@ -165,25 +165,16 @@ qemuDomainExtractTLSSubject(const char *certdir) virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot load cert data from %s: %s"), certfile, gnutls_strerror(ret)); - goto error; + return NULL; } subjectlen = 1024; - if (VIR_ALLOC_N(subject, subjectlen+1) < 0) - goto error; + subject = g_new0(char, subjectlen + 1); gnutls_x509_crt_get_dn(cert, subject, &subjectlen); subject[subjectlen] = '\0'; - VIR_FREE(certfile); - VIR_FREE(pemdata); - return subject; - - error: - VIR_FREE(certfile); - VIR_FREE(pemdata); - return NULL; } -- 2.26.2
Register the the cleanup functions for 'qemuMigrationCookieGraphics', 'qemuMigrationCookieNetwork', 'qemuMigrationCookieNBD', and 'qemuMigrationCookieCaps'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 006869a618..b71ff7bf9d 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -64,6 +64,9 @@ qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap) VIR_FREE(grap); } +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMigrationCookieGraphics, + qemuMigrationCookieGraphicsFree); + static void qemuMigrationCookieNetworkFree(qemuMigrationCookieNetworkPtr network) @@ -81,6 +84,8 @@ qemuMigrationCookieNetworkFree(qemuMigrationCookieNetworkPtr network) VIR_FREE(network); } +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMigrationCookieNetwork, + qemuMigrationCookieNetworkFree); static void qemuMigrationCookieNBDFree(qemuMigrationCookieNBDPtr nbd) @@ -94,6 +99,8 @@ qemuMigrationCookieNBDFree(qemuMigrationCookieNBDPtr nbd) VIR_FREE(nbd); } +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMigrationCookieNBD, + qemuMigrationCookieNBDFree); static void qemuMigrationCookieCapsFree(qemuMigrationCookieCapsPtr caps) @@ -106,6 +113,8 @@ qemuMigrationCookieCapsFree(qemuMigrationCookieCapsPtr caps) VIR_FREE(caps); } +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMigrationCookieCaps, + qemuMigrationCookieCapsFree); void qemuMigrationCookieFree(qemuMigrationCookiePtr mig) -- 2.26.2
Use modern memory handling approach to simplify the code. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index b71ff7bf9d..f63e3d7e12 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -192,13 +192,10 @@ qemuMigrationCookieGraphicsSpiceAlloc(virQEMUDriverPtr driver, virDomainGraphicsDefPtr def, virDomainGraphicsListenDefPtr glisten) { - qemuMigrationCookieGraphicsPtr mig = NULL; + g_autoptr(qemuMigrationCookieGraphics) mig = g_new0(qemuMigrationCookieGraphics, 1); const char *listenAddr; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - if (VIR_ALLOC(mig) < 0) - goto error; - mig->type = VIR_DOMAIN_GRAPHICS_TYPE_SPICE; mig->port = def->data.spice.port; if (cfg->spiceTLS) @@ -211,15 +208,11 @@ qemuMigrationCookieGraphicsSpiceAlloc(virQEMUDriverPtr driver, if (cfg->spiceTLS && !(mig->tlsSubject = qemuDomainExtractTLSSubject(cfg->spiceTLSx509certdir))) - goto error; + return NULL; mig->listen = g_strdup(listenAddr); - return mig; - - error: - qemuMigrationCookieGraphicsFree(mig); - return NULL; + return g_steal_pointer(&mig); } -- 2.26.2
Use modern memory handling approach to simplify the code. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index f63e3d7e12..7a1d115ae5 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -220,16 +220,11 @@ static qemuMigrationCookieNetworkPtr qemuMigrationCookieNetworkAlloc(virQEMUDriverPtr driver G_GNUC_UNUSED, virDomainDefPtr def) { - qemuMigrationCookieNetworkPtr mig; + g_autoptr(qemuMigrationCookieNetwork) mig = g_new0(qemuMigrationCookieNetwork, 1); size_t i; - if (VIR_ALLOC(mig) < 0) - goto error; - mig->nnets = def->nnets; - - if (VIR_ALLOC_N(mig->net, def->nnets) <0) - goto error; + mig->net = g_new0(qemuMigrationCookieNetData, def->nnets); for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr netptr; @@ -252,7 +247,7 @@ qemuMigrationCookieNetworkAlloc(virQEMUDriverPtr driver G_GNUC_UNUSED, virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to run command to get OVS port data for " "interface %s"), netptr->ifname); - goto error; + return NULL; } break; default: @@ -260,11 +255,7 @@ qemuMigrationCookieNetworkAlloc(virQEMUDriverPtr driver G_GNUC_UNUSED, } } } - return mig; - - error: - qemuMigrationCookieNetworkFree(mig); - return NULL; + return g_steal_pointer(&mig); } -- 2.26.2
Use automatic memory freeing to get rid of the 'error' label. Since the 'tmp' variable was used only in one instance, rename it appropriately. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 7a1d115ae5..8b5491e388 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -842,49 +842,40 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver, static qemuMigrationCookieGraphicsPtr qemuMigrationCookieGraphicsXMLParse(xmlXPathContextPtr ctxt) { - qemuMigrationCookieGraphicsPtr grap; - char *tmp; + g_autoptr(qemuMigrationCookieGraphics) grap = g_new0(qemuMigrationCookieGraphics, 1); + g_autofree char *graphicstype = NULL; - if (VIR_ALLOC(grap) < 0) - goto error; - - if (!(tmp = virXPathString("string(./graphics/@type)", ctxt))) { + if (!(graphicstype = virXPathString("string(./graphics/@type)", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing type attribute in migration data")); - goto error; + return NULL; } - if ((grap->type = virDomainGraphicsTypeFromString(tmp)) < 0) { + if ((grap->type = virDomainGraphicsTypeFromString(graphicstype)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown graphics type %s"), tmp); - VIR_FREE(tmp); - goto error; + _("unknown graphics type %s"), graphicstype); + return NULL; } - VIR_FREE(tmp); if (virXPathInt("string(./graphics/@port)", ctxt, &grap->port) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing port attribute in migration data")); - goto error; + return NULL; } if (grap->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { if (virXPathInt("string(./graphics/@tlsPort)", ctxt, &grap->tlsPort) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing tlsPort attribute in migration data")); - goto error; + return NULL; } } if (!(grap->listen = virXPathString("string(./graphics/@listen)", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing listen attribute in migration data")); - goto error; + return NULL; } /* Optional */ grap->tlsSubject = virXPathString("string(./graphics/cert[@info='subject']/@value)", ctxt); - return grap; - - error: - qemuMigrationCookieGraphicsFree(grap); - return NULL; + return g_steal_pointer(&grap); } -- 2.26.2
Use modern allocators, automatic memory feeing, and decrease the scope of some variables to remove the 'error' label. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 8b5491e388..c2ae3ab6a7 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -882,27 +882,24 @@ qemuMigrationCookieGraphicsXMLParse(xmlXPathContextPtr ctxt) static qemuMigrationCookieNetworkPtr qemuMigrationCookieNetworkXMLParse(xmlXPathContextPtr ctxt) { - qemuMigrationCookieNetworkPtr optr; + g_autoptr(qemuMigrationCookieNetwork) optr = g_new0(qemuMigrationCookieNetwork, 1); size_t i; int n; - xmlNodePtr *interfaces = NULL; - char *vporttype; + g_autofree xmlNodePtr *interfaces = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) - if (VIR_ALLOC(optr) < 0) - goto error; - if ((n = virXPathNodeSet("./network/interface", ctxt, &interfaces)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing interface information")); - goto error; + return NULL; } optr->nnets = n; - if (VIR_ALLOC_N(optr->net, optr->nnets) < 0) - goto error; + optr->net = g_new0(qemuMigrationCookieNetData, optr->nnets); for (i = 0; i < n; i++) { + g_autofree char *vporttype = NULL; + /* portdata is optional, and may not exist */ ctxt->node = interfaces[i]; optr->net[i].portdata = virXPathString("string(./portdata[1])", ctxt); @@ -910,20 +907,12 @@ qemuMigrationCookieNetworkXMLParse(xmlXPathContextPtr ctxt) if (!(vporttype = virXMLPropString(interfaces[i], "vporttype"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing vporttype attribute in migration data")); - goto error; + return NULL; } optr->net[i].vporttype = virNetDevVPortTypeFromString(vporttype); - VIR_FREE(vporttype); } - VIR_FREE(interfaces); - - return optr; - - error: - VIR_FREE(interfaces); - qemuMigrationCookieNetworkFree(optr); - return NULL; + return g_steal_pointer(&optr); } -- 2.26.2
Use modern allocators, automatic memory feeing, and decrease the scope of some variables to remove the 'error' and 'cleanup' labels. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index c2ae3ab6a7..1e0a1c3d7f 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -919,40 +919,37 @@ qemuMigrationCookieNetworkXMLParse(xmlXPathContextPtr ctxt) static qemuMigrationCookieNBDPtr qemuMigrationCookieNBDXMLParse(xmlXPathContextPtr ctxt) { - qemuMigrationCookieNBDPtr ret = NULL; - char *port = NULL, *capacity = NULL; + g_autoptr(qemuMigrationCookieNBD) ret = g_new0(qemuMigrationCookieNBD, 1); + g_autofree char *port = NULL; size_t i; int n; - xmlNodePtr *disks = NULL; + g_autofree xmlNodePtr *disks = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) - if (VIR_ALLOC(ret) < 0) - goto error; - port = virXPathString("string(./nbd/@port)", ctxt); if (port && virStrToLong_i(port, NULL, 10, &ret->port) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Malformed nbd port '%s'"), port); - goto error; + return NULL; } /* Now check if source sent a list of disks to prealloc. We might be * talking to an older server, so it's not an error if the list is * missing. */ if ((n = virXPathNodeSet("./nbd/disk", ctxt, &disks)) > 0) { - if (VIR_ALLOC_N(ret->disks, n) < 0) - goto error; + ret->disks = g_new0(struct qemuMigrationCookieNBDDisk, n); ret->ndisks = n; for (i = 0; i < n; i++) { + g_autofree char *capacity = NULL; + ctxt->node = disks[i]; - VIR_FREE(capacity); if (!(ret->disks[i].target = virXPathString("string(./@target)", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Malformed disk target")); - goto error; + return NULL; } capacity = virXPathString("string(./@capacity)", ctxt); @@ -962,20 +959,12 @@ qemuMigrationCookieNBDXMLParse(xmlXPathContextPtr ctxt) virReportError(VIR_ERR_INTERNAL_ERROR, _("Malformed disk capacity: '%s'"), NULLSTR(capacity)); - goto error; + return NULL; } } } - cleanup: - VIR_FREE(port); - VIR_FREE(capacity); - VIR_FREE(disks); - return ret; - error: - qemuMigrationCookieNBDFree(ret); - ret = NULL; - goto cleanup; + return g_steal_pointer(&ret); } -- 2.26.2
Use modern allocators, automatic memory feeing, and decrease the scope of some variables to remove the 'cleanup' label. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 31 +++++++++---------------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 1e0a1c3d7f..a1ef3d2b12 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -1057,29 +1057,26 @@ qemuMigrationCookieStatisticsXMLParse(xmlXPathContextPtr ctxt) static qemuMigrationCookieCapsPtr qemuMigrationCookieCapsXMLParse(xmlXPathContextPtr ctxt) { - qemuMigrationCookieCapsPtr caps = NULL; - xmlNodePtr *nodes = NULL; - qemuMigrationCookieCapsPtr ret = NULL; - char *name = NULL; - char *automatic = NULL; - int cap; + g_autoptr(qemuMigrationCookieCaps) caps = g_new0(qemuMigrationCookieCaps, 1); + g_autofree xmlNodePtr *nodes = NULL; size_t i; int n; - if (VIR_ALLOC(caps) < 0) - return NULL; - caps->supported = virBitmapNew(QEMU_MIGRATION_CAP_LAST); caps->automatic = virBitmapNew(QEMU_MIGRATION_CAP_LAST); if ((n = virXPathNodeSet("./capabilities[1]/cap", ctxt, &nodes)) < 0) - goto cleanup; + return NULL; for (i = 0; i < n; i++) { + g_autofree char *name = NULL; + g_autofree char *automatic = NULL; + int cap; + if (!(name = virXMLPropString(nodes[i], "name"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing migration capability name")); - goto cleanup; + return NULL; } if ((cap = qemuMigrationCapabilityTypeFromString(name)) < 0) @@ -1090,19 +1087,9 @@ qemuMigrationCookieCapsXMLParse(xmlXPathContextPtr ctxt) if ((automatic = virXMLPropString(nodes[i], "auto")) && STREQ(automatic, "yes")) ignore_value(virBitmapSetBit(caps->automatic, cap)); - - VIR_FREE(name); - VIR_FREE(automatic); } - ret = g_steal_pointer(&caps); - - cleanup: - qemuMigrationCookieCapsFree(caps); - VIR_FREE(nodes); - VIR_FREE(name); - VIR_FREE(automatic); - return ret; + return g_steal_pointer(&caps); } -- 2.26.2
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index a1ef3d2b12..93d91b8a1c 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -542,8 +542,7 @@ qemuMigrationCookieAddCaps(qemuMigrationCookiePtr mig, qemuDomainObjPrivatePtr priv = vm->privateData; qemuMigrationCookieCapsFree(mig->caps); - if (VIR_ALLOC(mig->caps) < 0) - return -1; + mig->caps = g_new0(qemuMigrationCookieCaps, 1); if (priv->migrationCaps) mig->caps->supported = virBitmapNewCopy(priv->migrationCaps); -- 2.26.2
Restructure the control-flow a bit using an temporary variable to avoid the need to use VIR_FREE. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 93d91b8a1c..84274301dc 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -1218,15 +1218,18 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig, if ((flags & QEMU_MIGRATION_COOKIE_LOCKSTATE) && virXPathBoolean("count(./lockstate) > 0", ctxt)) { + g_autofree char *lockState = NULL; + mig->lockDriver = virXPathString("string(./lockstate[1]/@driver)", ctxt); if (!mig->lockDriver) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing lock driver name in migration cookie")); return -1; } - mig->lockState = virXPathString("string(./lockstate[1]/leases[1])", ctxt); - if (mig->lockState && STREQ(mig->lockState, "")) - VIR_FREE(mig->lockState); + + lockState = virXPathString("string(./lockstate[1]/leases[1])", ctxt); + if (STRNEQ_NULLABLE(lockState, "")) + mig->lockState = g_steal_pointer(&lockState); } if ((flags & QEMU_MIGRATION_COOKIE_PERSISTENT) && -- 2.26.2
Previous refactors allow us to plainly replace all VIR_FREE by g_free to finish the modernization of the file. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration_cookie.c | 34 ++++++++++++++++---------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 84274301dc..1c792f15bc 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -59,9 +59,9 @@ qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap) { if (!grap) return; - VIR_FREE(grap->listen); - VIR_FREE(grap->tlsSubject); - VIR_FREE(grap); + g_free(grap->listen); + g_free(grap->tlsSubject); + g_free(grap); } G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMigrationCookieGraphics, @@ -78,10 +78,10 @@ qemuMigrationCookieNetworkFree(qemuMigrationCookieNetworkPtr network) if (network->net) { for (i = 0; i < network->nnets; i++) - VIR_FREE(network->net[i].portdata); + g_free(network->net[i].portdata); } - VIR_FREE(network->net); - VIR_FREE(network); + g_free(network->net); + g_free(network); } G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMigrationCookieNetwork, @@ -94,9 +94,9 @@ qemuMigrationCookieNBDFree(qemuMigrationCookieNBDPtr nbd) return; while (nbd->ndisks) - VIR_FREE(nbd->disks[--nbd->ndisks].target); - VIR_FREE(nbd->disks); - VIR_FREE(nbd); + g_free(nbd->disks[--nbd->ndisks].target); + g_free(nbd->disks); + g_free(nbd); } G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMigrationCookieNBD, @@ -110,7 +110,7 @@ qemuMigrationCookieCapsFree(qemuMigrationCookieCapsPtr caps) virBitmapFree(caps->supported); virBitmapFree(caps->automatic); - VIR_FREE(caps); + g_free(caps); } G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMigrationCookieCaps, @@ -127,15 +127,15 @@ qemuMigrationCookieFree(qemuMigrationCookiePtr mig) qemuMigrationCookieNetworkFree(mig->network); qemuMigrationCookieNBDFree(mig->nbd); - VIR_FREE(mig->localHostname); - VIR_FREE(mig->remoteHostname); - VIR_FREE(mig->name); - VIR_FREE(mig->lockState); - VIR_FREE(mig->lockDriver); + g_free(mig->localHostname); + g_free(mig->remoteHostname); + g_free(mig->name); + g_free(mig->lockState); + g_free(mig->lockDriver); g_clear_pointer(&mig->jobInfo, qemuDomainJobInfoFree); virCPUDefFree(mig->cpu); qemuMigrationCookieCapsFree(mig->caps); - VIR_FREE(mig); + g_free(mig); } @@ -1409,7 +1409,7 @@ qemuMigrationCookieParse(virQEMUDriverPtr driver, if (flags & QEMU_MIGRATION_COOKIE_PERSISTENT && mig->persistent && STRNEQ(def->name, mig->persistent->name)) { - VIR_FREE(mig->persistent->name); + g_free(mig->persistent->name); mig->persistent->name = g_strdup(def->name); } -- 2.26.2
On 10/2/20 10:57 AM, Peter Krempa wrote:
Note that this applies on top of the recent bitmap refactors
Peter Krempa (20): qemu: migration_cookie: Extract parsing/validation of mandatory features qemuMigrationCookieXMLParse: Switch to single-purpose temporary variables qemuMigrationCookieXMLParse: Check domain element count more defensively qemuMigrationCookieXMLParse: Decrease scope of 'nodes' and use automatic freeing qemuMigrationCookieXMLParse: Remove comment mentioning that error was already set qemuMigrationCookieXMLParse: Remove 'error' label qemuMigrationCookieGraphicsXMLFormat: Use 'virXMLFormatElement' qemuMigrationCookieNetworkXMLFormat: Refactor XML formatting qemuMigrationCookieXMLFormat: Extract formatting of NBD qemuDomainExtractTLSSubject: Refactor memory handling qemu: migration_cookie: Register 'autoptr' functions for internal types qemuMigrationCookieGraphicsSpiceAlloc: Refactor memory handling qemuMigrationCookieNetworkAlloc: Refactor memory handling qemuMigrationCookieXMLFormat: Refactor memory handling qemuMigrationCookieNetworkXMLParse: Refactor memory handling qemuMigrationCookieNBDXMLParse: Refactor memory handling qemuMigrationCookieCapsXMLParse: Refactor memory handling qemuMigrationCookieAddCaps: Use 'g_new0' instead of VIR_ALLOC qemuMigrationCookieXMLParse: Avoid VIR_FREE when parsing lockstate qemu: migration_cookie: s/VIR_FREE/g_free/
src/qemu/qemu_migration_cookie.c | 493 ++++++++++++++----------------- 1 file changed, 214 insertions(+), 279 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Prívozník -
Peter Krempa