Upcoming patches will add new flags for increasing the amount of
information present in <domain> dumpxml, but where the source
of that information is tied to somewhere other than the active
or offline domain sub-definition. Make those extensions easier
by updating internal callers to pass in a struct, rather than
adding new parameters for each extension, so that later patches
only have to patch callers that care about a new member of the
struct rather than all callers.
I considered updating virDomainDefFormat(), but as there were
so many existing callers, it was easier to just add a new
wrapper function virDomainDefFormatFull() which takes the full
struct, while making the existing interface forward on to the
full one.
Since all callers are being adjusted anyway, reorder the
parameters of virDomainDefFormatInternal to put buf first, as
that tends to be more typical.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
src/conf/domain_conf.h | 18 ++++++++++++++----
src/conf/domain_conf.c | 33 ++++++++++++++++++++++++++-------
src/conf/snapshot_conf.c | 5 ++++-
src/network/bridge_driver.c | 2 +-
src/qemu/qemu_domain.c | 12 ++++++------
5 files changed, 51 insertions(+), 19 deletions(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c2dcc87ba1..5086bc342a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1,7 +1,7 @@
/*
* domain_conf.h: domain XML processing
*
- * Copyright (C) 2006-2016 Red Hat, Inc.
+ * Copyright (C) 2006-2019 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
* Copyright (c) 2015 SUSE LINUX Products GmbH, Nuernberg, Germany.
*
@@ -3194,17 +3194,27 @@ void virDomainIOThreadIDDel(virDomainDefPtr def, unsigned int iothread_id);
VIR_DOMAIN_XML_MIGRATABLE)
unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags);
+/* Struct of extra information that may be required while formatting
+ * domains, according to the flags in use. */
+typedef struct _virDomainDefFormatData {
+ virCapsPtr caps;
+} virDomainDefFormatData;
+typedef struct _virDomainDefFormatData *virDomainDefFormatDataPtr;
+
char *virDomainDefFormat(virDomainDefPtr def,
virCapsPtr caps,
unsigned int flags);
+char *virDomainDefFormatFull(virDomainDefPtr def,
+ virDomainDefFormatDataPtr data,
+ unsigned int flags);
char *virDomainObjFormat(virDomainXMLOptionPtr xmlopt,
virDomainObjPtr obj,
virCapsPtr caps,
unsigned int flags);
-int virDomainDefFormatInternal(virDomainDefPtr def,
- virCapsPtr caps,
+int virDomainDefFormatInternal(virBufferPtr buf,
+ virDomainDefPtr def,
+ virDomainDefFormatDataPtr data,
unsigned int flags,
- virBufferPtr buf,
virDomainXMLOptionPtr xmlopt);
int virDomainDiskSourceFormat(virBufferPtr buf,
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bfafac407e..941d582dc1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -27826,14 +27826,15 @@ virDomainVsockDefFormat(virBufferPtr buf,
/* This internal version appends to an existing buffer
* (possibly with auto-indent), rather than flattening
* to string.
- * Return -1 on failure. */
+ * Return -1 on failure, with the buffer reset. */
int
-virDomainDefFormatInternal(virDomainDefPtr def,
- virCapsPtr caps,
+virDomainDefFormatInternal(virBufferPtr buf,
+ virDomainDefPtr def,
+ virDomainDefFormatDataPtr data,
unsigned int flags,
- virBufferPtr buf,
virDomainXMLOptionPtr xmlopt)
{
+ virCapsPtr caps = data ? data->caps : NULL;
unsigned char *uuid;
char uuidstr[VIR_UUID_STRING_BUFLEN];
const char *type = NULL;
@@ -28678,12 +28679,27 @@ unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags)
char *
-virDomainDefFormat(virDomainDefPtr def, virCapsPtr caps, unsigned int flags)
+virDomainDefFormat(virDomainDefPtr def,
+ virCapsPtr caps,
+ unsigned int flags)
+{
+ virDomainDefFormatData data = {
+ .caps = caps,
+ };
+
+ return virDomainDefFormatFull(def, &data, flags);
+}
+
+
+char *
+virDomainDefFormatFull(virDomainDefPtr def,
+ virDomainDefFormatDataPtr data,
+ unsigned int flags)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
virCheckFlags(VIR_DOMAIN_DEF_FORMAT_COMMON_FLAGS, NULL);
- if (virDomainDefFormatInternal(def, caps, flags, &buf, NULL) < 0)
+ if (virDomainDefFormatInternal(&buf, def, data, flags, NULL) < 0)
return NULL;
return virBufferContentAndReset(&buf);
@@ -28700,6 +28716,9 @@ virDomainObjFormat(virDomainXMLOptionPtr xmlopt,
int state;
int reason;
size_t i;
+ virDomainDefFormatData data = {
+ .caps = caps,
+ };
state = virDomainObjGetState(obj, &reason);
virBufferAsprintf(&buf, "<domstatus state='%s' reason='%s' pid='%lld'>\n",
@@ -28718,7 +28737,7 @@ virDomainObjFormat(virDomainXMLOptionPtr xmlopt,
xmlopt->privateData.format(&buf, obj) < 0)
goto error;
- if (virDomainDefFormatInternal(obj->def, caps, flags, &buf, xmlopt) < 0)
+ if (virDomainDefFormatInternal(&buf, obj->def, &data, flags, xmlopt) < 0)
goto error;
virBufferAdjustIndent(&buf, -2);
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 5641e57e43..206b05c172 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -715,6 +715,9 @@ virDomainSnapshotDefFormatInternal(virBufferPtr buf,
{
size_t i;
int domainflags = VIR_DOMAIN_DEF_FORMAT_INACTIVE;
+ virDomainDefFormatData data = {
+ .caps = caps,
+ };
if (flags & VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE)
domainflags |= VIR_DOMAIN_DEF_FORMAT_SECURE;
@@ -759,7 +762,7 @@ virDomainSnapshotDefFormatInternal(virBufferPtr buf,
}
if (def->dom) {
- if (virDomainDefFormatInternal(def->dom, caps, domainflags, buf,
+ if (virDomainDefFormatInternal(buf, def->dom, &data, domainflags,
xmlopt) < 0)
goto error;
} else if (uuidstr) {
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index b3ca5b8a15..86048168e9 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -226,7 +226,7 @@ networkRunHook(virNetworkObjPtr obj,
goto cleanup;
if (virNetworkDefFormatBuf(&buf, def, 0) < 0)
goto cleanup;
- if (dom && virDomainDefFormatInternal(dom, NULL, 0, &buf, NULL) < 0)
+ if (dom && virDomainDefFormatInternal(&buf, dom, NULL, 0, NULL) < 0)
goto cleanup;
virBufferAdjustIndent(&buf, -2);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f7353a9fac..db25e1596c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7724,18 +7724,18 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,
{
int ret = -1;
virDomainDefPtr copy = NULL;
- virCapsPtr caps = NULL;
virQEMUCapsPtr qemuCaps = NULL;
+ virDomainDefFormatData data = { 0 };
virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS | VIR_DOMAIN_XML_UPDATE_CPU, -1);
- if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+ if (!(data.caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
if (!(flags & (VIR_DOMAIN_XML_UPDATE_CPU | VIR_DOMAIN_XML_MIGRATABLE)))
goto format;
- if (!(copy = virDomainDefCopy(def, caps, driver->xmlopt, NULL,
+ if (!(copy = virDomainDefCopy(def, data.caps, driver->xmlopt, NULL,
flags & VIR_DOMAIN_XML_MIGRATABLE)))
goto cleanup;
@@ -7894,13 +7894,13 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,
}
format:
- ret = virDomainDefFormatInternal(def, caps,
+ ret = virDomainDefFormatInternal(buf, def, &data,
virDomainDefFormatConvertXMLFlags(flags),
- buf, driver->xmlopt);
+ driver->xmlopt);
cleanup:
virDomainDefFree(copy);
- virObjectUnref(caps);
+ virObjectUnref(data.caps);
virObjectUnref(qemuCaps);
return ret;
}
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 3/4/19 10:34 PM, Eric Blake wrote: > Upcoming patches will add new flags for increasing the amount of > information present in <domain> dumpxml, but where the source > of that information is tied to somewhere other than the active > or offline domain sub-definition. Make those extensions easier > by updating internal callers to pass in a struct, rather than > adding new parameters for each extension, so that later patches > only have to patch callers that care about a new member of the > struct rather than all callers. > > I considered updating virDomainDefFormat(), but as there were > so many existing callers, it was easier to just add a new > wrapper function virDomainDefFormatFull() which takes the full > struct, while making the existing interface forward on to the > full one. > > Since all callers are being adjusted anyway, reorder the > parameters of virDomainDefFormatInternal to put buf first, as > that tends to be more typical. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > src/conf/domain_conf.h | 18 ++++++++++++++---- > src/conf/domain_conf.c | 33 ++++++++++++++++++++++++++------- > src/conf/snapshot_conf.c | 5 ++++- > src/network/bridge_driver.c | 2 +- > src/qemu/qemu_domain.c | 12 ++++++------ > 5 files changed, 51 insertions(+), 19 deletions(-) > The structure formatting of _virDomainDefFormatData and friends is a bit non-standard from what is typically done, but not incorrect. Reviewed-by: John Ferlan <jferlan@redhat.com> John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 3/7/19 8:01 AM, John Ferlan wrote: >> Upcoming patches will add new flags for increasing the amount of >> information present in <domain> dumpxml, but where the source >> of that information is tied to somewhere other than the active >> or offline domain sub-definition. Make those extensions easier >> by updating internal callers to pass in a struct, rather than >> adding new parameters for each extension, so that later patches >> only have to patch callers that care about a new member of the >> struct rather than all callers. >> > > The structure formatting of _virDomainDefFormatData and friends is a bit > non-standard from what is typically done, but not incorrect. Do you have a pointer to a struct I should be copying from? I don't mind making that sort of cosmetic cleanup for code-base consistency. -- 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
On 3/7/19 1:14 PM, Eric Blake wrote:
> On 3/7/19 8:01 AM, John Ferlan wrote:
>
>>> Upcoming patches will add new flags for increasing the amount of
>>> information present in <domain> dumpxml, but where the source
>>> of that information is tied to somewhere other than the active
>>> or offline domain sub-definition. Make those extensions easier
>>> by updating internal callers to pass in a struct, rather than
>>> adding new parameters for each extension, so that later patches
>>> only have to patch callers that care about a new member of the
>>> struct rather than all callers.
>>>
>
>>
>> The structure formatting of _virDomainDefFormatData and friends is a bit
>> non-standard from what is typically done, but not incorrect.
>
> Do you have a pointer to a struct I should be copying from? I don't mind
> making that sort of cosmetic cleanup for code-base consistency.
>
Usually it's like...
typedef struct _virDomainDef virDomainDef;
typedef virDomainDef *virDomainDefPtr;
struct _virDomainDef {
...
};
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.