[libvirt] [PATCH 1/9] conf: eliminate hardcoded indent from all xml

Many of the domain xml format functions (including all of the device format functions) had hard-coded spaces, which made for incorrect indentation when those functions were called in a different context (for example, commit 2122cf39 added <interface> XML into the document provided to a network hook script, and in this case it should have been indented by 2 spaces, but was instead indented by 6 spaces). In that patch I mentioned doing a followup patch to make the device xml formatters more consistent. After doing that patch, it felt incomplete to not give the same treatment to the entire directory. The one downside to this series is that it may create merge conflicts during backports, but fortunately the conflicts should all be fairly easy to resolve. Laine Stump (9): conf: eliminate hardcoded indent from domain xml conf: eliminate hardcoded indent from domain snapshot xml conf: eliminate hardcoded indent from network xml conf: eliminate outmoded/odd indent method from interface xml conf: eliminate hardcoded indentation in nwfilter xml conf: eliminate hardcoded indentation in capabilities xml conf: eliminate hardcoded indentation in node device xml conf: eliminate hardcoded indent in volume/pool xml conf: eliminate hardcoded indentation in all remaining xml src/conf/capabilities.c | 183 ++++++----- src/conf/cpu_conf.c | 11 +- src/conf/domain_conf.c | 599 +++++++++++++++++++---------------- src/conf/interface_conf.c | 137 ++++---- src/conf/netdev_bandwidth_conf.c | 6 +- src/conf/netdev_vlan_conf.c | 6 +- src/conf/netdev_vport_profile_conf.c | 6 +- src/conf/network_conf.c | 8 +- src/conf/node_device_conf.c | 207 ++++++------ src/conf/nwfilter_conf.c | 94 ++---- src/conf/nwfilter_params.c | 6 +- src/conf/secret_conf.c | 20 +- src/conf/snapshot_conf.c | 48 +-- src/conf/storage_conf.c | 174 +++++----- src/conf/storage_encryption_conf.c | 6 +- 15 files changed, 811 insertions(+), 700 deletions(-) -- 1.8.5.3

Many of the domain xml format functions (including all of the device format functions) had hard-coded spaces, which made for incorrect indentation when those functions were called in a different context (for example, commit 2122cf39 added <interface> XML into the document provided to a network hook script, and in this case it should have been indented by 2 spaces, but was instead indented by 6 spaces). To make it possible to insert a properly indented device anywhere into an XML document, this patch removes hardcoded spaces from the formatting functions, and calls virBufferAdjustIndent() at appropriate places instead. (a regex search of domain_conf.c was done to assure that all occurrences of hardcoded spaces were removed). virDomainDiskSourceDefFormatInternal() is also called from snapshot_conf.c, so two virBufferAdjustIndent() calls were temporarily added around that call - those functions will have hardcoded spaces removed in a separate patch. This could cause some conflicts when backporting future changes to the formatting functions to older branches, but fortunately the changes are almost all trivial, so conflict resolution will be obvious. --- src/conf/domain_conf.c | 599 ++++++++++++++++++++++++++--------------------- src/conf/snapshot_conf.c | 4 +- 2 files changed, 336 insertions(+), 267 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1d5cc14..82642c9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3154,22 +3154,22 @@ virDomainDeviceInfoFormat(virBufferPtr buf, unsigned int flags) { if ((flags & VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT) && info->bootIndex) - virBufferAsprintf(buf, " <boot order='%d'/>\n", info->bootIndex); + virBufferAsprintf(buf, "<boot order='%d'/>\n", info->bootIndex); if (info->alias && !(flags & VIR_DOMAIN_XML_INACTIVE)) { - virBufferAsprintf(buf, " <alias name='%s'/>\n", info->alias); + virBufferAsprintf(buf, "<alias name='%s'/>\n", info->alias); } if (info->mastertype == VIR_DOMAIN_CONTROLLER_MASTER_USB) { - virBufferAsprintf(buf, " <master startport='%d'/>\n", + virBufferAsprintf(buf, "<master startport='%d'/>\n", info->master.usb.startport); } if ((flags & VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM) && (info->rombar || info->romfile)) { - virBufferAddLit(buf, " <rom"); + virBufferAddLit(buf, "<rom"); if (info->rombar) { const char *rombar = virDomainPciRombarModeTypeToString(info->rombar); @@ -3192,7 +3192,7 @@ virDomainDeviceInfoFormat(virBufferPtr buf, return 0; /* We'll be in domain/devices/[device type]/ so 3 level indent */ - virBufferAsprintf(buf, " <address type='%s'", + virBufferAsprintf(buf, "<address type='%s'", virDomainDeviceAddressTypeToString(info->type)); switch (info->type) { @@ -3264,7 +3264,6 @@ virDomainDeviceInfoFormat(virBufferPtr buf, } virBufferAddLit(buf, "/>\n"); - return 0; } @@ -14590,7 +14589,7 @@ virDomainEventActionDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <%s>%s</%s>\n", name, typeStr, name); + virBufferAsprintf(buf, "<%s>%s</%s>\n", name, typeStr, name); return 0; } @@ -14633,15 +14632,16 @@ virSecurityLabelDefFormat(virBufferPtr buf, if (def->label || def->imagelabel || def->baselabel) { virBufferAddLit(buf, ">\n"); - - virBufferEscapeString(buf, " <label>%s</label>\n", + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<label>%s</label>\n", def->label); if (!def->norelabel) - virBufferEscapeString(buf, " <imagelabel>%s</imagelabel>\n", + virBufferEscapeString(buf, "<imagelabel>%s</imagelabel>\n", def->imagelabel); if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) - virBufferEscapeString(buf, " <baselabel>%s</baselabel>\n", + virBufferEscapeString(buf, "<baselabel>%s</baselabel>\n", def->baselabel); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</seclabel>\n"); } else { virBufferAddLit(buf, "/>\n"); @@ -14671,8 +14671,10 @@ virSecurityDeviceLabelDefFormat(virBufferPtr buf, if (def->label) { virBufferAddLit(buf, ">\n"); - virBufferEscapeString(buf, " <label>%s</label>\n", + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<label>%s</label>\n", def->label); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</seclabel>\n"); } else { virBufferAddLit(buf, "/>\n"); @@ -14684,14 +14686,16 @@ static int virDomainLeaseDefFormat(virBufferPtr buf, virDomainLeaseDefPtr def) { - virBufferAddLit(buf, " <lease>\n"); - virBufferEscapeString(buf, " <lockspace>%s</lockspace>\n", def->lockspace); - virBufferEscapeString(buf, " <key>%s</key>\n", def->key); - virBufferEscapeString(buf, " <target path='%s'", def->path); + virBufferAddLit(buf, "<lease>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<lockspace>%s</lockspace>\n", def->lockspace); + virBufferEscapeString(buf, "<key>%s</key>\n", def->key); + virBufferEscapeString(buf, "<target path='%s'", def->path); if (def->offset) virBufferAsprintf(buf, " offset='%llu'", def->offset); virBufferAddLit(buf, "/>\n"); - virBufferAddLit(buf, " </lease>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</lease>\n"); return 0; } @@ -14707,7 +14711,7 @@ virDomainDiskGeometryDefFormat(virBufferPtr buf, def->geometry.heads > 0 && def->geometry.sectors > 0) { virBufferAsprintf(buf, - " <geometry cyls='%u' heads='%u' secs='%u'", + "<geometry cyls='%u' heads='%u' secs='%u'", def->geometry.cylinders, def->geometry.heads, def->geometry.sectors); @@ -14725,7 +14729,7 @@ virDomainDiskBlockIoDefFormat(virBufferPtr buf, { if (def->blockio.logical_block_size > 0 || def->blockio.physical_block_size > 0) { - virBufferAddLit(buf, " <blockio"); + virBufferAddLit(buf, "<blockio"); if (def->blockio.logical_block_size > 0) { virBufferAsprintf(buf, " logical_block_size='%u'", @@ -14756,11 +14760,11 @@ virDomainDiskSourceDefFormatSeclabel(virBufferPtr buf, if (nseclabels) { virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 8); + virBufferAdjustIndent(buf, 2); for (n = 0; n < nseclabels; n++) virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags); - virBufferAdjustIndent(buf, -8); - virBufferAddLit(buf, " </source>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</source>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -14788,7 +14792,7 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf, if (src || nhosts > 0 || srcpool || startupPolicy) { switch (type) { case VIR_DOMAIN_DISK_TYPE_FILE: - virBufferAddLit(buf, " <source"); + virBufferAddLit(buf, "<source"); virBufferEscapeString(buf, " file='%s'", src); virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); @@ -14796,7 +14800,7 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf, break; case VIR_DOMAIN_DISK_TYPE_BLOCK: - virBufferAddLit(buf, " <source"); + virBufferAddLit(buf, "<source"); virBufferEscapeString(buf, " dev='%s'", src); virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); @@ -14804,14 +14808,14 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf, break; case VIR_DOMAIN_DISK_TYPE_DIR: - virBufferAddLit(buf, " <source"); + virBufferAddLit(buf, "<source"); virBufferEscapeString(buf, " dir='%s'", src); virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); virBufferAddLit(buf, "/>\n"); break; case VIR_DOMAIN_DISK_TYPE_NETWORK: - virBufferAsprintf(buf, " <source protocol='%s'", + virBufferAsprintf(buf, "<source protocol='%s'", virDomainDiskProtocolTypeToString(protocol)); virBufferEscapeString(buf, " name='%s'", src); @@ -14819,8 +14823,9 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } else { virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); for (n = 0; n < nhosts; n++) { - virBufferAddLit(buf, " <host"); + virBufferAddLit(buf, "<host"); virBufferEscapeString(buf, " name='%s'", hosts[n].name); virBufferEscapeString(buf, " port='%s'", hosts[n].port); @@ -14832,12 +14837,13 @@ virDomainDiskSourceDefFormatInternal(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } - virBufferAddLit(buf, " </source>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</source>\n"); } break; case VIR_DOMAIN_DISK_TYPE_VOLUME: - virBufferAddLit(buf, " <source"); + virBufferAddLit(buf, "<source"); if (srcpool) { virBufferAsprintf(buf, " pool='%s' volume='%s'", @@ -14934,7 +14940,7 @@ virDomainDiskDefFormat(virBufferPtr buf, } virBufferAsprintf(buf, - " <disk type='%s' device='%s'", + "<disk type='%s' device='%s'", type, device); if (def->rawio_specified) { if (def->rawio == 1) { @@ -14952,10 +14958,11 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " snapshot='%s'", virDomainSnapshotLocationTypeToString(def->snapshot)); virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); if (def->driverName || def->format > 0 || def->cachemode || def->ioeventfd || def->event_idx || def->copy_on_read) { - virBufferAddLit(buf, " <driver"); + virBufferAddLit(buf, "<driver"); if (def->driverName) virBufferAsprintf(buf, " name='%s'", def->driverName); if (def->format > 0) @@ -14981,12 +14988,13 @@ virDomainDiskDefFormat(virBufferPtr buf, } if (def->auth.username) { - virBufferEscapeString(buf, " <auth username='%s'>\n", + virBufferEscapeString(buf, "<auth username='%s'>\n", def->auth.username); + virBufferAdjustIndent(buf, 2); if (def->protocol == VIR_DOMAIN_DISK_PROTOCOL_ISCSI) { - virBufferAddLit(buf, " <secret type='iscsi'"); + virBufferAddLit(buf, "<secret type='iscsi'"); } else if (def->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) { - virBufferAddLit(buf, " <secret type='ceph'"); + virBufferAddLit(buf, "<secret type='ceph'"); } if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_UUID) { @@ -14997,7 +15005,8 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " usage='%s'/>\n", def->auth.secret.usage); } - virBufferAddLit(buf, " </auth>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</auth>\n"); } if (virDomainDiskSourceDefFormat(buf, def, flags) < 0) @@ -15009,7 +15018,7 @@ virDomainDiskDefFormat(virBufferPtr buf, * for live domains, therefore we ignore it on input except for * the internal parse on libvirtd restart. */ if (def->mirror && !(flags & VIR_DOMAIN_XML_INACTIVE)) { - virBufferEscapeString(buf, " <mirror file='%s'", def->mirror); + virBufferEscapeString(buf, "<mirror file='%s'", def->mirror); if (def->mirrorFormat) virBufferAsprintf(buf, " format='%s'", virStorageFileFormatTypeToString(def->mirrorFormat)); @@ -15018,7 +15027,7 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } - virBufferAsprintf(buf, " <target dev='%s' bus='%s'", + virBufferAsprintf(buf, "<target dev='%s' bus='%s'", def->dst, bus); if ((def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY || def->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && @@ -15039,64 +15048,60 @@ virDomainDiskDefFormat(virBufferPtr buf, def->blkdeviotune.total_iops_sec || def->blkdeviotune.read_iops_sec || def->blkdeviotune.write_iops_sec) { - virBufferAddLit(buf, " <iotune>\n"); + virBufferAddLit(buf, "<iotune>\n"); + virBufferAdjustIndent(buf, 2); if (def->blkdeviotune.total_bytes_sec) { - virBufferAsprintf(buf, " <total_bytes_sec>%llu</total_bytes_sec>\n", + virBufferAsprintf(buf, "<total_bytes_sec>%llu</total_bytes_sec>\n", def->blkdeviotune.total_bytes_sec); } if (def->blkdeviotune.read_bytes_sec) { - virBufferAsprintf(buf, " <read_bytes_sec>%llu</read_bytes_sec>\n", + virBufferAsprintf(buf, "<read_bytes_sec>%llu</read_bytes_sec>\n", def->blkdeviotune.read_bytes_sec); } if (def->blkdeviotune.write_bytes_sec) { - virBufferAsprintf(buf, " <write_bytes_sec>%llu</write_bytes_sec>\n", + virBufferAsprintf(buf, "<write_bytes_sec>%llu</write_bytes_sec>\n", def->blkdeviotune.write_bytes_sec); } if (def->blkdeviotune.total_iops_sec) { - virBufferAsprintf(buf, " <total_iops_sec>%llu</total_iops_sec>\n", + virBufferAsprintf(buf, "<total_iops_sec>%llu</total_iops_sec>\n", def->blkdeviotune.total_iops_sec); } if (def->blkdeviotune.read_iops_sec) { - virBufferAsprintf(buf, " <read_iops_sec>%llu</read_iops_sec>\n", + virBufferAsprintf(buf, "<read_iops_sec>%llu</read_iops_sec>\n", def->blkdeviotune.read_iops_sec); } if (def->blkdeviotune.write_iops_sec) { - virBufferAsprintf(buf, " <write_iops_sec>%llu</write_iops_sec>\n", + virBufferAsprintf(buf, "<write_iops_sec>%llu</write_iops_sec>\n", def->blkdeviotune.write_iops_sec); } - - virBufferAddLit(buf, " </iotune>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</iotune>\n"); } if (def->readonly) - virBufferAddLit(buf, " <readonly/>\n"); + virBufferAddLit(buf, "<readonly/>\n"); if (def->shared) - virBufferAddLit(buf, " <shareable/>\n"); + virBufferAddLit(buf, "<shareable/>\n"); if (def->transient) - virBufferAddLit(buf, " <transient/>\n"); - virBufferEscapeString(buf, " <serial>%s</serial>\n", def->serial); - virBufferEscapeString(buf, " <wwn>%s</wwn>\n", def->wwn); - virBufferEscapeString(buf, " <vendor>%s</vendor>\n", def->vendor); - virBufferEscapeString(buf, " <product>%s</product>\n", def->product); - if (def->encryption) { - virBufferAdjustIndent(buf, 6); - if (virStorageEncryptionFormat(buf, def->encryption) < 0) - return -1; - virBufferAdjustIndent(buf, -6); - } - + virBufferAddLit(buf, "<transient/>\n"); + virBufferEscapeString(buf, "<serial>%s</serial>\n", def->serial); + virBufferEscapeString(buf, "<wwn>%s</wwn>\n", def->wwn); + virBufferEscapeString(buf, "<vendor>%s</vendor>\n", def->vendor); + virBufferEscapeString(buf, "<product>%s</product>\n", def->product); + if (def->encryption && virStorageEncryptionFormat(buf, def->encryption) < 0) + return -1; if (virDomainDeviceInfoFormat(buf, &def->info, flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT) < 0) return -1; - virBufferAddLit(buf, " </disk>\n"); - + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</disk>\n"); return 0; } @@ -15140,7 +15145,7 @@ virDomainControllerDefFormat(virBufferPtr buf, } virBufferAsprintf(buf, - " <controller type='%s' index='%u'", + "<controller type='%s' index='%u'", type, def->idx); if (model) { @@ -15171,20 +15176,21 @@ virDomainControllerDefFormat(virBufferPtr buf, if (def->queues || virDomainDeviceInfoIsSet(&def->info, flags) || pcihole64) { virBufferAddLit(buf, ">\n"); - + virBufferAdjustIndent(buf, 2); if (def->queues) - virBufferAsprintf(buf, " <driver queues='%u'/>\n", def->queues); + virBufferAsprintf(buf, "<driver queues='%u'/>\n", def->queues); if (virDomainDeviceInfoIsSet(&def->info, flags) && virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; if (pcihole64) { - virBufferAsprintf(buf, " <pcihole64 unit='KiB'>%lu</" + virBufferAsprintf(buf, "<pcihole64 unit='KiB'>%lu</" "pcihole64>\n", def->opts.pciopts.pcihole64size); } - virBufferAddLit(buf, " </controller>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</controller>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -15232,11 +15238,11 @@ virDomainFSDefFormat(virBufferPtr buf, virBufferAsprintf(buf, - " <filesystem type='%s' accessmode='%s'>\n", + "<filesystem type='%s' accessmode='%s'>\n", type, accessmode); - + virBufferAdjustIndent(buf, 2); if (def->fsdriver) { - virBufferAsprintf(buf, " <driver type='%s'", fsdriver); + virBufferAsprintf(buf, "<driver type='%s'", fsdriver); if (def->format) virBufferAsprintf(buf, " format='%s'", @@ -15252,50 +15258,50 @@ virDomainFSDefFormat(virBufferPtr buf, switch (def->type) { case VIR_DOMAIN_FS_TYPE_MOUNT: case VIR_DOMAIN_FS_TYPE_BIND: - virBufferEscapeString(buf, " <source dir='%s'/>\n", + virBufferEscapeString(buf, "<source dir='%s'/>\n", def->src); break; case VIR_DOMAIN_FS_TYPE_BLOCK: - virBufferEscapeString(buf, " <source dev='%s'/>\n", + virBufferEscapeString(buf, "<source dev='%s'/>\n", def->src); break; case VIR_DOMAIN_FS_TYPE_FILE: - virBufferEscapeString(buf, " <source file='%s'/>\n", + virBufferEscapeString(buf, "<source file='%s'/>\n", def->src); break; case VIR_DOMAIN_FS_TYPE_TEMPLATE: - virBufferEscapeString(buf, " <source name='%s'/>\n", + virBufferEscapeString(buf, "<source name='%s'/>\n", def->src); break; case VIR_DOMAIN_FS_TYPE_RAM: - virBufferAsprintf(buf, " <source usage='%lld' units='KiB'/>\n", + virBufferAsprintf(buf, "<source usage='%lld' units='KiB'/>\n", def->usage / 1024); break; } - virBufferEscapeString(buf, " <target dir='%s'/>\n", + virBufferEscapeString(buf, "<target dir='%s'/>\n", def->dst); if (def->readonly) - virBufferAddLit(buf, " <readonly/>\n"); + virBufferAddLit(buf, "<readonly/>\n"); if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; if (def->space_hard_limit) - virBufferAsprintf(buf, " <space_hard_limit unit='bytes'>" + virBufferAsprintf(buf, "<space_hard_limit unit='bytes'>" "%llu</space_hard_limit>\n", def->space_hard_limit); if (def->space_soft_limit) { - virBufferAsprintf(buf, " <space_soft_limit unit='bytes'>" + virBufferAsprintf(buf, "<space_soft_limit unit='bytes'>" "%llu</space_soft_limit>\n", def->space_soft_limit); } - virBufferAddLit(buf, " </filesystem>\n"); - + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</filesystem>\n"); return 0; } @@ -15364,12 +15370,14 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, def->origstates.states.pci.remove_slot || def->origstates.states.pci.reprobe)) { virBufferAddLit(buf, "<origstates>\n"); + virBufferAdjustIndent(buf, 2); if (def->origstates.states.pci.unbind_from_stub) - virBufferAddLit(buf, " <unbind/>\n"); + virBufferAddLit(buf, "<unbind/>\n"); if (def->origstates.states.pci.remove_slot) - virBufferAddLit(buf, " <removeslot/>\n"); + virBufferAddLit(buf, "<removeslot/>\n"); if (def->origstates.states.pci.reprobe) - virBufferAddLit(buf, " <reprobe/>\n"); + virBufferAddLit(buf, "<reprobe/>\n"); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</origstates>\n"); } break; @@ -15574,12 +15582,12 @@ virDomainNetDefFormat(virBufferPtr buf, hostdef = &def->data.hostdev.def; } - virBufferAsprintf(buf, " <interface type='%s'", typeStr); + virBufferAsprintf(buf, "<interface type='%s'", typeStr); if (hostdef && hostdef->managed) virBufferAddLit(buf, " managed='yes'"); virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 6); + virBufferAdjustIndent(buf, 2); virBufferAsprintf(buf, "<mac address='%s'/>\n", virMacAddrFormat(&def->mac, macstr)); @@ -15723,7 +15731,9 @@ virDomainNetDefFormat(virBufferPtr buf, if (def->tune.sndbuf_specified) { virBufferAddLit(buf, "<tune>\n"); - virBufferAsprintf(buf, " <sndbuf>%lu</sndbuf>\n", def->tune.sndbuf); + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<sndbuf>%lu</sndbuf>\n", def->tune.sndbuf); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</tune>\n"); } @@ -15731,16 +15741,13 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "<link state='%s'/>\n", virDomainNetInterfaceLinkStateTypeToString(def->linkstate)); } - - virBufferAdjustIndent(buf, -6); - if (virDomainDeviceInfoFormat(buf, &def->info, flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT | VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM) < 0) return -1; - virBufferAddLit(buf, " </interface>\n"); - + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</interface>\n"); return 0; } @@ -15784,7 +15791,7 @@ virDomainChrSourceDefFormat(virBufferPtr buf, if (def->type != VIR_DOMAIN_CHR_TYPE_PTY || (def->data.file.path && !(flags & VIR_DOMAIN_XML_INACTIVE))) { - virBufferEscapeString(buf, " <source path='%s'/>\n", + virBufferEscapeString(buf, "<source path='%s'/>\n", def->data.file.path); } break; @@ -15793,55 +15800,55 @@ virDomainChrSourceDefFormat(virBufferPtr buf, if (def->data.udp.bindService && def->data.udp.bindHost) { virBufferAsprintf(buf, - " <source mode='bind' host='%s' " + "<source mode='bind' host='%s' " "service='%s'/>\n", def->data.udp.bindHost, def->data.udp.bindService); } else if (def->data.udp.bindHost) { - virBufferAsprintf(buf, " <source mode='bind' host='%s'/>\n", + virBufferAsprintf(buf, "<source mode='bind' host='%s'/>\n", def->data.udp.bindHost); } else if (def->data.udp.bindService) { - virBufferAsprintf(buf, " <source mode='bind' service='%s'/>\n", + virBufferAsprintf(buf, "<source mode='bind' service='%s'/>\n", def->data.udp.bindService); } if (def->data.udp.connectService && def->data.udp.connectHost) { virBufferAsprintf(buf, - " <source mode='connect' host='%s' " + "<source mode='connect' host='%s' " "service='%s'/>\n", def->data.udp.connectHost, def->data.udp.connectService); } else if (def->data.udp.connectHost) { - virBufferAsprintf(buf, " <source mode='connect' host='%s'/>\n", + virBufferAsprintf(buf, "<source mode='connect' host='%s'/>\n", def->data.udp.connectHost); } else if (def->data.udp.connectService) { virBufferAsprintf(buf, - " <source mode='connect' service='%s'/>\n", + "<source mode='connect' service='%s'/>\n", def->data.udp.connectService); } break; case VIR_DOMAIN_CHR_TYPE_TCP: virBufferAsprintf(buf, - " <source mode='%s' host='%s' service='%s'/>\n", + "<source mode='%s' host='%s' service='%s'/>\n", def->data.tcp.listen ? "bind" : "connect", def->data.tcp.host, def->data.tcp.service); - virBufferAsprintf(buf, " <protocol type='%s'/>\n", + virBufferAsprintf(buf, "<protocol type='%s'/>\n", virDomainChrTcpProtocolTypeToString( def->data.tcp.protocol)); break; case VIR_DOMAIN_CHR_TYPE_UNIX: - virBufferAsprintf(buf, " <source mode='%s'", + virBufferAsprintf(buf, "<source mode='%s'", def->data.nix.listen ? "bind" : "connect"); virBufferEscapeString(buf, " path='%s'", def->data.nix.path); virBufferAddLit(buf, "/>\n"); break; case VIR_DOMAIN_CHR_TYPE_SPICEPORT: - virBufferAsprintf(buf, " <source channel='%s'/>\n", + virBufferAsprintf(buf, "<source channel='%s'/>\n", def->data.spiceport.channel); break; @@ -15870,7 +15877,8 @@ virDomainChrDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <%s", elementName); + virBufferAsprintf(buf, "<%s", elementName); + virBufferAdjustIndent(buf, 2); tty_compat = (def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && def->target.port == 0 && def->source.type == VIR_DOMAIN_CHR_TYPE_PTY && @@ -15887,7 +15895,7 @@ virDomainChrDefFormat(virBufferPtr buf, _("Could not format channel target type")); return -1; } - virBufferAsprintf(buf, " <target type='%s'", targetType); + virBufferAsprintf(buf, "<target type='%s'", targetType); switch (def->targetType) { case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD: { @@ -15922,7 +15930,7 @@ virDomainChrDefFormat(virBufferPtr buf, case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE: virBufferAsprintf(buf, - " <target type='%s' port='%d'/>\n", + "<target type='%s' port='%d'/>\n", virDomainChrTargetTypeToString(def->deviceType, def->targetType), def->target.port); @@ -15931,14 +15939,14 @@ virDomainChrDefFormat(virBufferPtr buf, case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: if (def->targetTypeAttr) { virBufferAsprintf(buf, - " <target type='%s' port='%d'/>\n", + "<target type='%s' port='%d'/>\n", virDomainChrTargetTypeToString(def->deviceType, def->targetType), def->target.port); break; } default: - virBufferAsprintf(buf, " <target port='%d'/>\n", + virBufferAsprintf(buf, "<target port='%d'/>\n", def->target.port); break; } @@ -15956,7 +15964,8 @@ virDomainChrDefFormat(virBufferPtr buf, virBufferAdjustIndent(buf, -2); } - virBufferAsprintf(buf, " </%s>\n", elementName); + virBufferAdjustIndent(buf, -2); + virBufferAsprintf(buf, "</%s>\n", elementName); return ret; } @@ -15975,10 +15984,12 @@ virDomainSmartcardDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <smartcard mode='%s'", mode); + virBufferAsprintf(buf, "<smartcard mode='%s'", mode); + virBufferAdjustIndent(buf, 2); switch (def->type) { case VIR_DOMAIN_SMARTCARD_TYPE_HOST: if (!virDomainDeviceInfoIsSet(&def->info, flags)) { + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "/>\n"); return 0; } @@ -15988,9 +15999,9 @@ virDomainSmartcardDefFormat(virBufferPtr buf, case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: virBufferAddLit(buf, ">\n"); for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) - virBufferEscapeString(buf, " <certificate>%s</certificate>\n", + virBufferEscapeString(buf, "<certificate>%s</certificate>\n", def->data.cert.file[i]); - virBufferEscapeString(buf, " <database>%s</database>\n", + virBufferEscapeString(buf, "<database>%s</database>\n", def->data.cert.database); break; @@ -16007,7 +16018,8 @@ virDomainSmartcardDefFormat(virBufferPtr buf, } if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; - virBufferAddLit(buf, " </smartcard>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</smartcard>\n"); return 0; } @@ -16023,7 +16035,7 @@ virDomainSoundCodecDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <codec type='%s'/>\n", type); + virBufferAsprintf(buf, "<codec type='%s'/>\n", type); return 0; } @@ -16033,29 +16045,32 @@ virDomainTPMDefFormat(virBufferPtr buf, virDomainTPMDefPtr def, unsigned int flags) { - virBufferAsprintf(buf, " <tpm model='%s'>\n", + virBufferAsprintf(buf, "<tpm model='%s'>\n", virDomainTPMModelTypeToString(def->model)); - - virBufferAsprintf(buf, " <backend type='%s'>\n", + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<backend type='%s'>\n", virDomainTPMBackendTypeToString(def->type)); + virBufferAdjustIndent(buf, 2); switch (def->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: - virBufferEscapeString(buf, " <device path='%s'/>\n", + virBufferEscapeString(buf, "<device path='%s'/>\n", def->data.passthrough.source.data.file.path); break; case VIR_DOMAIN_TPM_TYPE_LAST: break; } - virBufferAddLit(buf, " </backend>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</backend>\n"); if (virDomainDeviceInfoIsSet(&def->info, flags)) { if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; } - virBufferAddLit(buf, " </tpm>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</tpm>\n"); return 0; } @@ -16076,11 +16091,12 @@ virDomainSoundDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <sound model='%s'", model); + virBufferAsprintf(buf, "<sound model='%s'", model); for (i = 0; i < def->ncodecs; i++) { if (!children) { virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); children = true; } virDomainSoundCodecDefFormat(buf, def->codecs[i]); @@ -16089,6 +16105,7 @@ virDomainSoundDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info, flags)) { if (!children) { virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); children = true; } if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) @@ -16096,7 +16113,8 @@ virDomainSoundDefFormat(virBufferPtr buf, } if (children) { - virBufferAddLit(buf, " </sound>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</sound>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -16119,7 +16137,8 @@ virDomainMemballoonDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <memballoon model='%s'", model); + virBufferAsprintf(buf, "<memballoon model='%s'", model); + virBufferAdjustIndent(buf, 2); if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); @@ -16131,14 +16150,15 @@ virDomainMemballoonDefFormat(virBufferPtr buf, if (def->period) { if (noopts) virBufferAddLit(buf, ">\n"); - virBufferAsprintf(buf, " <stats period='%u'/>\n", def->period); + virBufferAsprintf(buf, "<stats period='%u'/>\n", def->period); noopts = false; } + virBufferAdjustIndent(buf, -2); if (noopts) virBufferAddLit(buf, "/>\n"); else - virBufferAddLit(buf, " </memballoon>\n"); + virBufferAddLit(buf, "</memballoon>\n"); return 0; } @@ -16148,12 +16168,14 @@ virDomainNVRAMDefFormat(virBufferPtr buf, virDomainNVRAMDefPtr def, unsigned int flags) { - virBufferAddLit(buf, " <nvram>\n"); + virBufferAddLit(buf, "<nvram>\n"); + virBufferAdjustIndent(buf, 2); if (virDomainDeviceInfoIsSet(&def->info, flags) && virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; - virBufferAddLit(buf, " </nvram>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</nvram>\n"); return 0; } @@ -16190,14 +16212,16 @@ virDomainWatchdogDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <watchdog model='%s' action='%s'", + virBufferAsprintf(buf, "<watchdog model='%s' action='%s'", model, action); if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; - virBufferAddLit(buf, " </watchdog>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</watchdog>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -16208,10 +16232,12 @@ virDomainWatchdogDefFormat(virBufferPtr buf, static int virDomainPanicDefFormat(virBufferPtr buf, virDomainPanicDefPtr def) { - virBufferAddLit(buf, " <panic>\n"); + virBufferAddLit(buf, "<panic>\n"); + virBufferAdjustIndent(buf, 2); if (virDomainDeviceInfoFormat(buf, &def->info, 0) < 0) return -1; - virBufferAddLit(buf, " </panic>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</panic>\n"); return 0; } @@ -16224,14 +16250,15 @@ virDomainRNGDefFormat(virBufferPtr buf, const char *model = virDomainRNGModelTypeToString(def->model); const char *backend = virDomainRNGBackendTypeToString(def->backend); - virBufferAsprintf(buf, " <rng model='%s'>\n", model); + virBufferAsprintf(buf, "<rng model='%s'>\n", model); + virBufferAdjustIndent(buf, 2); if (def->rate) { - virBufferAsprintf(buf, " <rate bytes='%u'", def->rate); + virBufferAsprintf(buf, "<rate bytes='%u'", def->rate); if (def->period) virBufferAsprintf(buf, " period='%u'", def->period); virBufferAddLit(buf, "/>\n"); } - virBufferAsprintf(buf, " <backend model='%s'", backend); + virBufferAsprintf(buf, "<backend model='%s'", backend); switch ((enum virDomainRNGBackend) def->backend) { case VIR_DOMAIN_RNG_BACKEND_RANDOM: @@ -16248,7 +16275,7 @@ virDomainRNGDefFormat(virBufferPtr buf, false, flags) < 0) return -1; virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, " </backend>\n"); + virBufferAddLit(buf, "</backend>\n"); case VIR_DOMAIN_RNG_BACKEND_LAST: break; @@ -16259,8 +16286,8 @@ virDomainRNGDefFormat(virBufferPtr buf, return -1; } - virBufferAddLit(buf, " </rng>\n"); - + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</rng>\n"); return 0; } @@ -16289,7 +16316,7 @@ static void virDomainVideoAccelDefFormat(virBufferPtr buf, virDomainVideoAccelDefPtr def) { - virBufferAsprintf(buf, " <acceleration accel3d='%s'", + virBufferAsprintf(buf, "<acceleration accel3d='%s'", def->support3d ? "yes" : "no"); virBufferAsprintf(buf, " accel2d='%s'", def->support2d ? "yes" : "no"); @@ -16310,8 +16337,9 @@ virDomainVideoDefFormat(virBufferPtr buf, return -1; } - virBufferAddLit(buf, " <video>\n"); - virBufferAsprintf(buf, " <model type='%s'", + virBufferAddLit(buf, "<video>\n"); + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<model type='%s'", model); if (def->ram) virBufferAsprintf(buf, " ram='%u'", def->ram); @@ -16323,8 +16351,10 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAddLit(buf, " primary='yes'"); if (def->accel) { virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); virDomainVideoAccelDefFormat(buf, def->accel); - virBufferAddLit(buf, " </model>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</model>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -16332,8 +16362,8 @@ virDomainVideoDefFormat(virBufferPtr buf, if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; - virBufferAddLit(buf, " </video>\n"); - + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</video>\n"); return 0; } @@ -16356,14 +16386,16 @@ virDomainInputDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <input type='%s' bus='%s'", + virBufferAsprintf(buf, "<input type='%s' bus='%s'", type, bus); if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; - virBufferAddLit(buf, " </input>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</input>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -16383,7 +16415,7 @@ virDomainTimerDefFormat(virBufferPtr buf, _("unexpected timer name %d"), def->name); return -1; } - virBufferAsprintf(buf, " <timer name='%s'", name); + virBufferAsprintf(buf, "<timer name='%s'", name); if (def->present == 0) { virBufferAddLit(buf, " present='no'"); @@ -16441,7 +16473,8 @@ virDomainTimerDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } else { virBufferAddLit(buf, ">\n"); - virBufferAddLit(buf, " <catchup"); + virBufferAdjustIndent(buf, 2); + virBufferAddLit(buf, "<catchup"); if (def->catchup.threshold > 0) { virBufferAsprintf(buf, " threshold='%lu'", def->catchup.threshold); } @@ -16452,7 +16485,8 @@ virDomainTimerDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " limit='%lu'", def->catchup.limit); } virBufferAddLit(buf, "/>\n"); - virBufferAddLit(buf, " </timer>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</timer>\n"); } return 0; @@ -16494,8 +16528,7 @@ virDomainGraphicsListenDefFormat(virBufferPtr buf, if ((flags & VIR_DOMAIN_XML_MIGRATABLE) && def->fromConfig) return; - virBufferAddLit(buf, " <listen"); - + virBufferAddLit(buf, "<listen"); if (def->type) { virBufferAsprintf(buf, " type='%s'", virDomainGraphicsListenTypeToString(def->type)); @@ -16552,7 +16585,7 @@ virDomainGraphicsDefFormat(virBufferPtr buf, } } - virBufferAsprintf(buf, " <graphics type='%s'", type); + virBufferAsprintf(buf, "<graphics type='%s'", type); switch (def->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: @@ -16671,6 +16704,7 @@ virDomainGraphicsDefFormat(virBufferPtr buf, continue; if (!children) { virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); children = true; } virDomainGraphicsListenDefFormat(buf, &def->listens[i], flags); @@ -16684,10 +16718,11 @@ virDomainGraphicsDefFormat(virBufferPtr buf, if (!children) { virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); children = true; } - virBufferAsprintf(buf, " <channel name='%s' mode='%s'/>\n", + virBufferAsprintf(buf, "<channel name='%s' mode='%s'/>\n", virDomainGraphicsSpiceChannelNameTypeToString(i), virDomainGraphicsSpiceChannelModeTypeToString(mode)); } @@ -16696,36 +16731,38 @@ virDomainGraphicsDefFormat(virBufferPtr buf, def->data.spice.streaming || def->data.spice.copypaste || def->data.spice.mousemode || def->data.spice.filetransfer)) { virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); children = true; } if (def->data.spice.image) - virBufferAsprintf(buf, " <image compression='%s'/>\n", + virBufferAsprintf(buf, "<image compression='%s'/>\n", virDomainGraphicsSpiceImageCompressionTypeToString(def->data.spice.image)); if (def->data.spice.jpeg) - virBufferAsprintf(buf, " <jpeg compression='%s'/>\n", + virBufferAsprintf(buf, "<jpeg compression='%s'/>\n", virDomainGraphicsSpiceJpegCompressionTypeToString(def->data.spice.jpeg)); if (def->data.spice.zlib) - virBufferAsprintf(buf, " <zlib compression='%s'/>\n", + virBufferAsprintf(buf, "<zlib compression='%s'/>\n", virDomainGraphicsSpiceZlibCompressionTypeToString(def->data.spice.zlib)); if (def->data.spice.playback) - virBufferAsprintf(buf, " <playback compression='%s'/>\n", + virBufferAsprintf(buf, "<playback compression='%s'/>\n", virDomainGraphicsSpicePlaybackCompressionTypeToString(def->data.spice.playback)); if (def->data.spice.streaming) - virBufferAsprintf(buf, " <streaming mode='%s'/>\n", + virBufferAsprintf(buf, "<streaming mode='%s'/>\n", virDomainGraphicsSpiceStreamingModeTypeToString(def->data.spice.streaming)); if (def->data.spice.mousemode) - virBufferAsprintf(buf, " <mouse mode='%s'/>\n", + virBufferAsprintf(buf, "<mouse mode='%s'/>\n", virDomainGraphicsSpiceMouseModeTypeToString(def->data.spice.mousemode)); if (def->data.spice.copypaste) - virBufferAsprintf(buf, " <clipboard copypaste='%s'/>\n", + virBufferAsprintf(buf, "<clipboard copypaste='%s'/>\n", virDomainGraphicsSpiceClipboardCopypasteTypeToString(def->data.spice.copypaste)); if (def->data.spice.filetransfer) - virBufferAsprintf(buf, " <filetransfer enable='%s'/>\n", + virBufferAsprintf(buf, "<filetransfer enable='%s'/>\n", virDomainGraphicsSpiceAgentFileTransferTypeToString(def->data.spice.filetransfer)); } if (children) { - virBufferAddLit(buf, " </graphics>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</graphics>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -16773,7 +16810,7 @@ virDomainHostdevDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <hostdev mode='%s' type='%s'", + virBufferAsprintf(buf, "<hostdev mode='%s' type='%s'", mode, type); if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { virBufferAsprintf(buf, " managed='%s'", @@ -16786,8 +16823,8 @@ virDomainHostdevDefFormat(virBufferPtr buf, virDomainDeviceSGIOTypeToString(def->source.subsys.u.scsi.sgio)); } virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); - virBufferAdjustIndent(buf, 6); switch (def->mode) { case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS: if (virDomainHostdevDefFormatSubsys(buf, def, flags, false) < 0) @@ -16803,14 +16840,14 @@ virDomainHostdevDefFormat(virBufferPtr buf, virBufferAddLit(buf, "<readonly/>\n"); if (def->shareable) virBufferAddLit(buf, "<shareable/>\n"); - virBufferAdjustIndent(buf, -6); if (virDomainDeviceInfoFormat(buf, def->info, flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT | VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM) < 0) return -1; - virBufferAddLit(buf, " </hostdev>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</hostdev>\n"); return 0; } @@ -16824,14 +16861,15 @@ virDomainRedirdevDefFormat(virBufferPtr buf, bus = virDomainRedirdevBusTypeToString(def->bus); - virBufferAsprintf(buf, " <redirdev bus='%s'", bus); + virBufferAsprintf(buf, "<redirdev bus='%s'", bus); + virBufferAdjustIndent(buf, 2); if (virDomainChrSourceDefFormat(buf, &def->source.chr, false, flags) < 0) return -1; if (virDomainDeviceInfoFormat(buf, &def->info, flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT) < 0) return -1; - virBufferAddLit(buf, " </redirdev>\n"); - + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</redirdev>\n"); return 0; } @@ -16841,10 +16879,11 @@ virDomainRedirFilterDefFormat(virBufferPtr buf, { size_t i; - virBufferAddLit(buf, " <redirfilter>\n"); + virBufferAddLit(buf, "<redirfilter>\n"); + virBufferAdjustIndent(buf, 2); for (i = 0; i < filter->nusbdevs; i++) { virDomainRedirFilterUsbDevDefPtr usbdev = filter->usbdevs[i]; - virBufferAddLit(buf, " <usbdev"); + virBufferAddLit(buf, "<usbdev"); if (usbdev->usbClass >= 0) virBufferAsprintf(buf, " class='0x%02X'", usbdev->usbClass); @@ -16864,7 +16903,8 @@ virDomainRedirFilterDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " allow='%s'/>\n", usbdev->allow ? "yes" : "no"); } - virBufferAddLit(buf, " </redirfilter>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</redirfilter>\n"); return 0; } @@ -16881,13 +16921,15 @@ virDomainHubDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <hub type='%s'", type); + virBufferAsprintf(buf, "<hub type='%s'", type); if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; - virBufferAddLit(buf, " </hub>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</hub>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -16926,9 +16968,11 @@ static void virDomainResourceDefFormat(virBufferPtr buf, virDomainResourceDefPtr def) { - virBufferAddLit(buf, " <resource>\n"); - virBufferEscapeString(buf, " <partition>%s</partition>\n", def->partition); - virBufferAddLit(buf, " </resource>\n"); + virBufferAddLit(buf, "<resource>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<partition>%s</partition>\n", def->partition); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</resource>\n"); } @@ -16982,16 +17026,17 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->namespaceData && def->ns.href) virBufferAsprintf(buf, " %s", (def->ns.href)()); virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); - virBufferEscapeString(buf, " <name>%s</name>\n", def->name); + virBufferEscapeString(buf, "<name>%s</name>\n", def->name); uuid = def->uuid; virUUIDFormat(uuid, uuidstr); - virBufferAsprintf(buf, " <uuid>%s</uuid>\n", uuidstr); + virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuidstr); - virBufferEscapeString(buf, " <title>%s</title>\n", def->title); + virBufferEscapeString(buf, "<title>%s</title>\n", def->title); - virBufferEscapeString(buf, " <description>%s</description>\n", + virBufferEscapeString(buf, "<description>%s</description>\n", def->description); if (def->metadata) { @@ -17007,24 +17052,24 @@ virDomainDefFormatInternal(virDomainDefPtr def, xmlIndentTreeOutput = 1; xmlbuf = xmlBufferCreate(); if (xmlNodeDump(xmlbuf, def->metadata->doc, def->metadata, - virBufferGetIndent(buf, false) / 2 + 1, 1) < 0) { + virBufferGetIndent(buf, false) / 2, 1) < 0) { xmlBufferFree(xmlbuf); xmlIndentTreeOutput = oldIndentTreeOutput; goto error; } - virBufferAsprintf(buf, " %s\n", (char *) xmlBufferContent(xmlbuf)); + virBufferAsprintf(buf, "%s\n", (char *) xmlBufferContent(xmlbuf)); xmlBufferFree(xmlbuf); xmlIndentTreeOutput = oldIndentTreeOutput; } - virBufferAddLit(buf, " <memory"); + virBufferAddLit(buf, "<memory"); if (def->mem.dump_core) virBufferAsprintf(buf, " dumpCore='%s'", virDomainMemDumpTypeToString(def->mem.dump_core)); virBufferAsprintf(buf, " unit='KiB'>%llu</memory>\n", def->mem.max_balloon); - virBufferAsprintf(buf, " <currentMemory unit='KiB'>%llu</currentMemory>\n", + virBufferAsprintf(buf, "<currentMemory unit='KiB'>%llu</currentMemory>\n", def->mem.cur_balloon); /* add blkiotune only if there are any */ @@ -17044,10 +17089,11 @@ virDomainDefFormatInternal(virDomainDefPtr def, } if (blkio) { - virBufferAddLit(buf, " <blkiotune>\n"); + virBufferAddLit(buf, "<blkiotune>\n"); + virBufferAdjustIndent(buf, 2); if (def->blkio.weight) - virBufferAsprintf(buf, " <weight>%u</weight>\n", + virBufferAsprintf(buf, "<weight>%u</weight>\n", def->blkio.weight); for (n = 0; n < def->blkio.ndevices; n++) { @@ -17056,28 +17102,31 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (!dev->weight && !dev->riops && !dev->wiops && !dev->rbps && !dev->wbps) continue; - virBufferAddLit(buf, " <device>\n"); - virBufferEscapeString(buf, " <path>%s</path>\n", + virBufferAddLit(buf, "<device>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<path>%s</path>\n", dev->path); if (dev->weight) - virBufferAsprintf(buf, " <weight>%u</weight>\n", + virBufferAsprintf(buf, "<weight>%u</weight>\n", dev->weight); if (dev->riops) - virBufferAsprintf(buf, " <read_iops_sec>%u</read_iops_sec>\n", + virBufferAsprintf(buf, "<read_iops_sec>%u</read_iops_sec>\n", dev->riops); if (dev->wiops) - virBufferAsprintf(buf, " <write_iops_sec>%u</write_iops_sec>\n", + virBufferAsprintf(buf, "<write_iops_sec>%u</write_iops_sec>\n", dev->wiops); if (dev->rbps) - virBufferAsprintf(buf, " <read_bytes_sec>%llu</read_bytes_sec>\n", + virBufferAsprintf(buf, "<read_bytes_sec>%llu</read_bytes_sec>\n", dev->rbps); if (dev->wbps) - virBufferAsprintf(buf, " <write_bytes_sec>%llu</write_bytes_sec>\n", + virBufferAsprintf(buf, "<write_bytes_sec>%llu</write_bytes_sec>\n", dev->wbps); - virBufferAddLit(buf, " </device>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</device>\n"); } - virBufferAddLit(buf, " </blkiotune>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</blkiotune>\n"); } /* add memtune only if there are any */ @@ -17088,38 +17137,42 @@ virDomainDefFormatInternal(virDomainDefPtr def, (def->mem.swap_hard_limit && def->mem.hard_limit != VIR_DOMAIN_MEMORY_PARAM_UNLIMITED) || def->mem.min_guarantee) { - virBufferAddLit(buf, " <memtune>\n"); + virBufferAddLit(buf, "<memtune>\n"); + virBufferAdjustIndent(buf, 2); if (def->mem.hard_limit) { - virBufferAsprintf(buf, " <hard_limit unit='KiB'>" + virBufferAsprintf(buf, "<hard_limit unit='KiB'>" "%llu</hard_limit>\n", def->mem.hard_limit); } if (def->mem.soft_limit) { - virBufferAsprintf(buf, " <soft_limit unit='KiB'>" + virBufferAsprintf(buf, "<soft_limit unit='KiB'>" "%llu</soft_limit>\n", def->mem.soft_limit); } if (def->mem.min_guarantee) { - virBufferAsprintf(buf, " <min_guarantee unit='KiB'>" + virBufferAsprintf(buf, "<min_guarantee unit='KiB'>" "%llu</min_guarantee>\n", def->mem.min_guarantee); } if (def->mem.swap_hard_limit) { - virBufferAsprintf(buf, " <swap_hard_limit unit='KiB'>" + virBufferAsprintf(buf, "<swap_hard_limit unit='KiB'>" "%llu</swap_hard_limit>\n", def->mem.swap_hard_limit); } - virBufferAddLit(buf, " </memtune>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</memtune>\n"); } if (def->mem.hugepage_backed || def->mem.nosharepages || def->mem.locked) { - virBufferAddLit(buf, " <memoryBacking>\n"); + virBufferAddLit(buf, "<memoryBacking>\n"); + virBufferAdjustIndent(buf, 2); if (def->mem.hugepage_backed) - virBufferAddLit(buf, " <hugepages/>\n"); + virBufferAddLit(buf, "<hugepages/>\n"); if (def->mem.nosharepages) - virBufferAddLit(buf, " <nosharepages/>\n"); + virBufferAddLit(buf, "<nosharepages/>\n"); if (def->mem.locked) - virBufferAddLit(buf, " <locked/>\n"); - virBufferAddLit(buf, " </memoryBacking>\n"); + virBufferAddLit(buf, "<locked/>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</memoryBacking>\n"); } - virBufferAddLit(buf, " <vcpu"); + virBufferAddLit(buf, "<vcpu"); virBufferAsprintf(buf, " placement='%s'", virDomainCpuPlacementModeTypeToString(def->placement_mode)); @@ -17139,25 +17192,26 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->cputune.period || def->cputune.quota || def->cputune.emulatorpin || def->cputune.emulator_period || def->cputune.emulator_quota) - virBufferAddLit(buf, " <cputune>\n"); + virBufferAddLit(buf, "<cputune>\n"); + virBufferAdjustIndent(buf, 2); if (def->cputune.shares) - virBufferAsprintf(buf, " <shares>%lu</shares>\n", + virBufferAsprintf(buf, "<shares>%lu</shares>\n", def->cputune.shares); if (def->cputune.period) - virBufferAsprintf(buf, " <period>%llu</period>\n", + virBufferAsprintf(buf, "<period>%llu</period>\n", def->cputune.period); if (def->cputune.quota) - virBufferAsprintf(buf, " <quota>%lld</quota>\n", + virBufferAsprintf(buf, "<quota>%lld</quota>\n", def->cputune.quota); if (def->cputune.emulator_period) - virBufferAsprintf(buf, " <emulator_period>%llu" + virBufferAsprintf(buf, "<emulator_period>%llu" "</emulator_period>\n", def->cputune.emulator_period); if (def->cputune.emulator_quota) - virBufferAsprintf(buf, " <emulator_quota>%lld" + virBufferAsprintf(buf, "<emulator_quota>%lld" "</emulator_quota>\n", def->cputune.emulator_quota); @@ -17169,7 +17223,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->cputune.vcpupin[i]->cpumask)) continue; - virBufferAsprintf(buf, " <vcpupin vcpu='%u' ", + virBufferAsprintf(buf, "<vcpupin vcpu='%u' ", def->cputune.vcpupin[i]->vcpuid); if (!(cpumask = virBitmapFormat(def->cputune.vcpupin[i]->cpumask))) { @@ -17184,7 +17238,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->cputune.emulatorpin) { char *cpumask; - virBufferAddLit(buf, " <emulatorpin "); + virBufferAddLit(buf, "<emulatorpin "); if (!(cpumask = virBitmapFormat(def->cputune.emulatorpin->cpumask))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -17195,22 +17249,24 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAsprintf(buf, "cpuset='%s'/>\n", cpumask); VIR_FREE(cpumask); } + virBufferAdjustIndent(buf, -2); if (def->cputune.shares || (def->cputune.nvcpupin && !virDomainIsAllVcpupinInherited(def)) || def->cputune.period || def->cputune.quota || def->cputune.emulatorpin || def->cputune.emulator_period || def->cputune.emulator_quota) - virBufferAddLit(buf, " </cputune>\n"); + virBufferAddLit(buf, "</cputune>\n"); if (def->numatune.memory.nodemask || def->numatune.memory.placement_mode) { - virBufferAddLit(buf, " <numatune>\n"); const char *mode; char *nodemask = NULL; const char *placement; + virBufferAddLit(buf, "<numatune>\n"); + virBufferAdjustIndent(buf, 2); mode = virDomainNumatuneMemModeTypeToString(def->numatune.memory.mode); - virBufferAsprintf(buf, " <memory mode='%s' ", mode); + virBufferAsprintf(buf, "<memory mode='%s' ", mode); if (def->numatune.memory.placement_mode == VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_STATIC) { @@ -17227,7 +17283,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, placement = virNumaTuneMemPlacementModeTypeToString(def->numatune.memory.placement_mode); virBufferAsprintf(buf, "placement='%s'/>\n", placement); } - virBufferAddLit(buf, " </numatune>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</numatune>\n"); } if (def->resource) @@ -17237,16 +17294,16 @@ virDomainDefFormatInternal(virDomainDefPtr def, virDomainSysinfoDefFormat(buf, def->sysinfo); if (def->os.bootloader) { - virBufferEscapeString(buf, " <bootloader>%s</bootloader>\n", + virBufferEscapeString(buf, "<bootloader>%s</bootloader>\n", def->os.bootloader); virBufferEscapeString(buf, - " <bootloader_args>%s</bootloader_args>\n", + "<bootloader_args>%s</bootloader_args>\n", def->os.bootloaderArgs); } - virBufferAddLit(buf, " <os>\n"); - - virBufferAddLit(buf, " <type"); + virBufferAddLit(buf, "<os>\n"); + virBufferAdjustIndent(buf, 2); + virBufferAddLit(buf, "<type"); if (def->os.arch) virBufferAsprintf(buf, " arch='%s'", virArchToString(def->os.arch)); if (def->os.machine) @@ -17262,22 +17319,22 @@ virDomainDefFormatInternal(virDomainDefPtr def, else virBufferAsprintf(buf, ">%s</type>\n", def->os.type); - virBufferEscapeString(buf, " <init>%s</init>\n", + virBufferEscapeString(buf, "<init>%s</init>\n", def->os.init); for (i = 0; def->os.initargv && def->os.initargv[i]; i++) - virBufferEscapeString(buf, " <initarg>%s</initarg>\n", + virBufferEscapeString(buf, "<initarg>%s</initarg>\n", def->os.initargv[i]); - virBufferEscapeString(buf, " <loader>%s</loader>\n", + virBufferEscapeString(buf, "<loader>%s</loader>\n", def->os.loader); - virBufferEscapeString(buf, " <kernel>%s</kernel>\n", + virBufferEscapeString(buf, "<kernel>%s</kernel>\n", def->os.kernel); - virBufferEscapeString(buf, " <initrd>%s</initrd>\n", + virBufferEscapeString(buf, "<initrd>%s</initrd>\n", def->os.initrd); - virBufferEscapeString(buf, " <cmdline>%s</cmdline>\n", + virBufferEscapeString(buf, "<cmdline>%s</cmdline>\n", def->os.cmdline); - virBufferEscapeString(buf, " <dtb>%s</dtb>\n", + virBufferEscapeString(buf, "<dtb>%s</dtb>\n", def->os.dtb); - virBufferEscapeString(buf, " <root>%s</root>\n", + virBufferEscapeString(buf, "<root>%s</root>\n", def->os.root); if (!def->os.bootloader) { @@ -17290,18 +17347,18 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->os.bootDevs[n]); goto error; } - virBufferAsprintf(buf, " <boot dev='%s'/>\n", boottype); + virBufferAsprintf(buf, "<boot dev='%s'/>\n", boottype); } if (def->os.bootmenu != VIR_DOMAIN_BOOT_MENU_DEFAULT) { const char *enabled = (def->os.bootmenu == VIR_DOMAIN_BOOT_MENU_ENABLED ? "yes" : "no"); - virBufferAsprintf(buf, " <bootmenu enable='%s'/>\n", enabled); + virBufferAsprintf(buf, "<bootmenu enable='%s'/>\n", enabled); } if (def->os.bios.useserial || def->os.bios.rt_set) { - virBufferAddLit(buf, " <bios"); + virBufferAddLit(buf, "<bios"); if (def->os.bios.useserial) virBufferAsprintf(buf, " useserial='%s'", (def->os.bios.useserial == @@ -17323,29 +17380,32 @@ virDomainDefFormatInternal(virDomainDefPtr def, _("unexpected smbios mode %d"), def->os.smbios_mode); goto error; } - virBufferAsprintf(buf, " <smbios mode='%s'/>\n", mode); + virBufferAsprintf(buf, "<smbios mode='%s'/>\n", mode); } - virBufferAddLit(buf, " </os>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</os>\n"); if (def->idmap.uidmap) { - virBufferAddLit(buf, " <idmap>\n"); + virBufferAddLit(buf, "<idmap>\n"); + virBufferAdjustIndent(buf, 2); for (i = 0; i < def->idmap.nuidmap; i++) { virBufferAsprintf(buf, - " <uid start='%u' target='%u' count='%u'/>\n", + "<uid start='%u' target='%u' count='%u'/>\n", def->idmap.uidmap[i].start, def->idmap.uidmap[i].target, def->idmap.uidmap[i].count); } for (i = 0; i < def->idmap.ngidmap; i++) { virBufferAsprintf(buf, - " <gid start='%u' target='%u' count='%u'/>\n", + "<gid start='%u' target='%u' count='%u'/>\n", def->idmap.gidmap[i].start, def->idmap.gidmap[i].target, def->idmap.gidmap[i].count); } - virBufferAddLit(buf, " </idmap>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</idmap>\n"); } for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) { @@ -17354,7 +17414,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, } if (i != VIR_DOMAIN_FEATURE_LAST) { - virBufferAddLit(buf, " <features>\n"); + virBufferAddLit(buf, "<features>\n"); + virBufferAdjustIndent(buf, 2); for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) { const char *name = virDomainFeatureTypeToString(i); @@ -17377,7 +17438,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, break; case VIR_DOMAIN_FEATURE_STATE_ON: - virBufferAsprintf(buf, " <%s/>\n", name); + virBufferAsprintf(buf, "<%s/>\n", name); break; case VIR_DOMAIN_FEATURE_STATE_LAST: @@ -17398,11 +17459,11 @@ virDomainDefFormatInternal(virDomainDefPtr def, break; case VIR_DOMAIN_FEATURE_STATE_ON: - virBufferAsprintf(buf, " <%s state='on'/>\n", name); + virBufferAsprintf(buf, "<%s state='on'/>\n", name); break; case VIR_DOMAIN_FEATURE_STATE_OFF: - virBufferAsprintf(buf, " <%s state='off'/>\n", name); + virBufferAsprintf(buf, "<%s state='off'/>\n", name); break; } @@ -17410,7 +17471,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, case VIR_DOMAIN_FEATURE_APIC: if (def->features[i] == VIR_DOMAIN_FEATURE_STATE_ON) { - virBufferAddLit(buf, " <apic"); + virBufferAddLit(buf, "<apic"); if (def->apic_eoi) { virBufferAsprintf(buf, " eoi='%s'", virDomainFeatureStateTypeToString(def->apic_eoi)); @@ -17423,13 +17484,14 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->features[i] != VIR_DOMAIN_FEATURE_STATE_ON) break; - virBufferAddLit(buf, " <hyperv>\n"); + virBufferAddLit(buf, "<hyperv>\n"); + virBufferAdjustIndent(buf, 2); for (j = 0; j < VIR_DOMAIN_HYPERV_LAST; j++) { switch ((enum virDomainHyperv) j) { case VIR_DOMAIN_HYPERV_RELAXED: case VIR_DOMAIN_HYPERV_VAPIC: if (def->hyperv_features[j]) - virBufferAsprintf(buf, " <%s state='%s'/>\n", + virBufferAsprintf(buf, "<%s state='%s'/>\n", virDomainHypervTypeToString(j), virDomainFeatureStateTypeToString( def->hyperv_features[j])); @@ -17439,7 +17501,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->hyperv_features[j] == 0) break; - virBufferAsprintf(buf, " <spinlocks state='%s'", + virBufferAsprintf(buf, "<spinlocks state='%s'", virDomainFeatureStateTypeToString( def->hyperv_features[j])); if (def->hyperv_features[j] == VIR_DOMAIN_FEATURE_STATE_ON) @@ -17452,7 +17514,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, break; } } - virBufferAddLit(buf, " </hyperv>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</hyperv>\n"); break; case VIR_DOMAIN_FEATURE_LAST: @@ -17460,15 +17523,14 @@ virDomainDefFormatInternal(virDomainDefPtr def, } } - virBufferAddLit(buf, " </features>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</features>\n"); } - virBufferAdjustIndent(buf, 2); if (virCPUDefFormatBufFull(buf, def->cpu, flags) < 0) goto error; - virBufferAdjustIndent(buf, -2); - virBufferAsprintf(buf, " <clock offset='%s'", + virBufferAsprintf(buf, "<clock offset='%s'", virDomainClockOffsetTypeToString(def->clock.offset)); switch (def->clock.offset) { case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME: @@ -17493,11 +17555,13 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, "/>\n"); } else { virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); for (n = 0; n < def->clock.ntimers; n++) { if (virDomainTimerDefFormat(buf, def->clock.timers[n]) < 0) goto error; } - virBufferAddLit(buf, " </clock>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</clock>\n"); } if (virDomainEventActionDefFormat(buf, def->onPoweroff, @@ -17519,21 +17583,24 @@ virDomainDefFormatInternal(virDomainDefPtr def, goto error; if (def->pm.s3 || def->pm.s4) { - virBufferAddLit(buf, " <pm>\n"); + virBufferAddLit(buf, "<pm>\n"); + virBufferAdjustIndent(buf, 2); if (def->pm.s3) { - virBufferAsprintf(buf, " <suspend-to-mem enabled='%s'/>\n", + virBufferAsprintf(buf, "<suspend-to-mem enabled='%s'/>\n", virDomainPMStateTypeToString(def->pm.s3)); } if (def->pm.s4) { - virBufferAsprintf(buf, " <suspend-to-disk enabled='%s'/>\n", + virBufferAsprintf(buf, "<suspend-to-disk enabled='%s'/>\n", virDomainPMStateTypeToString(def->pm.s4)); } - virBufferAddLit(buf, " </pm>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</pm>\n"); } - virBufferAddLit(buf, " <devices>\n"); + virBufferAddLit(buf, "<devices>\n"); + virBufferAdjustIndent(buf, 2); - virBufferEscapeString(buf, " <emulator>%s</emulator>\n", + virBufferEscapeString(buf, "<emulator>%s</emulator>\n", def->emulator); for (n = 0; n < def->ndisks; n++) @@ -17682,9 +17749,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, virDomainPanicDefFormat(buf, def->panic) < 0) goto error; - virBufferAddLit(buf, " </devices>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</devices>\n"); - virBufferAdjustIndent(buf, 2); for (n = 0; n < def->nseclabels; n++) virSecurityLabelDefFormat(buf, def->seclabels[n], flags); virBufferAdjustIndent(buf, -2); @@ -17736,10 +17803,11 @@ virDomainObjFormat(virDomainXMLOptionPtr xmlopt, virDomainStateTypeToString(state), virDomainStateReasonToString(state, reason), (long long)obj->pid); + virBufferAdjustIndent(&buf, 2); for (i = 0; i < VIR_DOMAIN_TAINT_LAST; i++) { if (obj->taint & (1 << i)) - virBufferAsprintf(&buf, " <taint flag='%s'/>\n", + virBufferAsprintf(&buf, "<taint flag='%s'/>\n", virDomainTaintTypeToString(i)); } @@ -17747,11 +17815,10 @@ virDomainObjFormat(virDomainXMLOptionPtr xmlopt, ((xmlopt->privateData.format)(&buf, obj->privateData)) < 0) goto error; - virBufferAdjustIndent(&buf, 2); if (virDomainDefFormatInternal(obj->def, flags, &buf) < 0) goto error; - virBufferAdjustIndent(&buf, -2); + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</domstatus>\n"); if (virBufferError(&buf)) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 12b0930..f2ad980 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -1,7 +1,7 @@ /* * snapshot_conf.c: domain snapshot XML processing * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -630,6 +630,7 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, if (disk->format > 0) virBufferEscapeString(buf, " <driver type='%s'/>\n", virStorageFileFormatTypeToString(disk->format)); + virBufferAdjustIndent(buf, 6); virDomainDiskSourceDefFormatInternal(buf, type, disk->file, @@ -639,6 +640,7 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, disk->hosts, 0, NULL, NULL, 0); + virBufferAdjustIndent(buf, -6); virBufferAddLit(buf, " </disk>\n"); } -- 1.8.5.3

On 03/06/2014 08:24 AM, Laine Stump wrote:
Many of the domain xml format functions (including all of the device format functions) had hard-coded spaces, which made for incorrect indentation when those functions were called in a different context (for example, commit 2122cf39 added <interface> XML into the document provided to a network hook script, and in this case it should have been indented by 2 spaces, but was instead indented by 6 spaces).
To make it possible to insert a properly indented device anywhere into an XML document, this patch removes hardcoded spaces from the formatting functions, and calls virBufferAdjustIndent() at appropriate places instead. (a regex search of domain_conf.c was done to assure that all occurrences of hardcoded spaces were removed).
virDomainDiskSourceDefFormatInternal() is also called from snapshot_conf.c, so two virBufferAdjustIndent() calls were temporarily added around that call - those functions will have hardcoded spaces removed in a separate patch.
This could cause some conflicts when backporting future changes to the formatting functions to older branches, but fortunately the changes are almost all trivial, so conflict resolution will be obvious. --- src/conf/domain_conf.c | 599 ++++++++++++++++++++++++++--------------------- src/conf/snapshot_conf.c | 4 +- 2 files changed, 336 insertions(+), 267 deletions(-)
@@ -14671,8 +14671,10 @@ virSecurityDeviceLabelDefFormat(virBufferPtr buf,
if (def->label) { virBufferAddLit(buf, ">\n"); - virBufferEscapeString(buf, " <label>%s</label>\n", + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<label>%s</label>\n", def->label); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</seclabel>\n");
For small hunks like this where all the context is local, this just adds lines of code. I'm almost of the opinion that it's easier to have each function start with no indent, and any strings it prints locally are correctly indented (allowing leading whitespace), while any helper functions it calls are surrounded by AdjustIndent (since the helper function also prints at a local top level).
} else { virBufferAddLit(buf, "/>\n"); @@ -14684,14 +14686,16 @@ static int virDomainLeaseDefFormat(virBufferPtr buf, virDomainLeaseDefPtr def) { - virBufferAddLit(buf, " <lease>\n"); - virBufferEscapeString(buf, " <lockspace>%s</lockspace>\n", def->lockspace); - virBufferEscapeString(buf, " <key>%s</key>\n", def->key); - virBufferEscapeString(buf, " <target path='%s'", def->path); + virBufferAddLit(buf, "<lease>\n");
That is, changes like this are good (this is the first print of the function, so it should be done as if at the top level)...
+ virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<lockspace>%s</lockspace>\n", def->lockspace); + virBufferEscapeString(buf, "<key>%s</key>\n", def->key); + virBufferEscapeString(buf, "<target path='%s'", def->path);
...while all these are local so using " <" would be just as easy to follow. On the other hand, your argument of consistency (that it is easier to grep for '" ' violations) is mechanically testable, while my idea of local indentation is not. So I can live with your patch. The clincher is that our testsuite validates that we didn't break any output :) ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 03/06/2014 07:15 PM, Eric Blake wrote:
On 03/06/2014 08:24 AM, Laine Stump wrote:
if (def->label) { virBufferAddLit(buf, ">\n"); - virBufferEscapeString(buf, " <label>%s</label>\n", + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<label>%s</label>\n", def->label); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</seclabel>\n");
For small hunks like this where all the context is local, this just adds lines of code. I'm almost of the opinion that it's easier to have each function start with no indent, and any strings it prints locally are correctly indented (allowing leading whitespace), while any helper functions it calls are surrounded by AdjustIndent (since the helper function also prints at a local top level).
Yeah, I thought of that too, but it made it easier to verify, as you say below, and once I got going it was difficult to pick a cutoff point, so I just did it 100%.
The clincher is that our testsuite validates that we didn't break any output :) ACK.
Definitely I wouldn't have been able to make these changes with any level of confidence without the tests (and yes, they did catch a few things I'd missed, e.g. accidentally adding indent instead of removing).

All leading spaces in domain snapshot xml format functions have been replaced with appropriate calls to virBufferAdjustIndent(). This will make it easier to call other similarly fixed format functions (e.g. domain device format functions). --- src/conf/snapshot_conf.c | 48 +++++++++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index f2ad980..d70d176 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -615,7 +615,7 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, if (!disk->name) return; - virBufferEscapeString(buf, " <disk name='%s'", disk->name); + virBufferEscapeString(buf, "<disk name='%s'", disk->name); if (disk->snapshot > 0) virBufferAsprintf(buf, " snapshot='%s'", virDomainSnapshotLocationTypeToString(disk->snapshot)); @@ -626,11 +626,11 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, } virBufferAsprintf(buf, " type='%s'>\n", virDomainDiskTypeToString(type)); + virBufferAdjustIndent(buf, 2); if (disk->format > 0) - virBufferEscapeString(buf, " <driver type='%s'/>\n", + virBufferEscapeString(buf, "<driver type='%s'/>\n", virStorageFileFormatTypeToString(disk->format)); - virBufferAdjustIndent(buf, 6); virDomainDiskSourceDefFormatInternal(buf, type, disk->file, @@ -640,8 +640,8 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, disk->hosts, 0, NULL, NULL, 0); - virBufferAdjustIndent(buf, -6); - virBufferAddLit(buf, " </disk>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</disk>\n"); } char *virDomainSnapshotDefFormat(const char *domain_uuid, @@ -658,45 +658,51 @@ char *virDomainSnapshotDefFormat(const char *domain_uuid, flags |= VIR_DOMAIN_XML_INACTIVE; virBufferAddLit(&buf, "<domainsnapshot>\n"); - virBufferEscapeString(&buf, " <name>%s</name>\n", def->name); + virBufferAdjustIndent(&buf, 2); + virBufferEscapeString(&buf, "<name>%s</name>\n", def->name); if (def->description) - virBufferEscapeString(&buf, " <description>%s</description>\n", + virBufferEscapeString(&buf, "<description>%s</description>\n", def->description); - virBufferAsprintf(&buf, " <state>%s</state>\n", + virBufferAsprintf(&buf, "<state>%s</state>\n", virDomainSnapshotStateTypeToString(def->state)); if (def->parent) { - virBufferAddLit(&buf, " <parent>\n"); - virBufferEscapeString(&buf, " <name>%s</name>\n", def->parent); - virBufferAddLit(&buf, " </parent>\n"); + virBufferAddLit(&buf, "<parent>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferEscapeString(&buf, "<name>%s</name>\n", def->parent); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</parent>\n"); } - virBufferAsprintf(&buf, " <creationTime>%lld</creationTime>\n", + virBufferAsprintf(&buf, "<creationTime>%lld</creationTime>\n", def->creationTime); if (def->memory) { - virBufferAsprintf(&buf, " <memory snapshot='%s'", + virBufferAsprintf(&buf, "<memory snapshot='%s'", virDomainSnapshotLocationTypeToString(def->memory)); virBufferEscapeString(&buf, " file='%s'", def->file); virBufferAddLit(&buf, "/>\n"); } if (def->ndisks) { - virBufferAddLit(&buf, " <disks>\n"); + virBufferAddLit(&buf, "<disks>\n"); + virBufferAdjustIndent(&buf, 2); for (i = 0; i < def->ndisks; i++) virDomainSnapshotDiskDefFormat(&buf, &def->disks[i]); - virBufferAddLit(&buf, " </disks>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</disks>\n"); } if (def->dom) { - virBufferAdjustIndent(&buf, 2); if (virDomainDefFormatInternal(def->dom, flags, &buf) < 0) { virBufferFreeAndReset(&buf); return NULL; } - virBufferAdjustIndent(&buf, -2); } else if (domain_uuid) { - virBufferAddLit(&buf, " <domain>\n"); - virBufferAsprintf(&buf, " <uuid>%s</uuid>\n", domain_uuid); - virBufferAddLit(&buf, " </domain>\n"); + virBufferAddLit(&buf, "<domain>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<uuid>%s</uuid>\n", domain_uuid); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</domain>\n"); } if (internal) - virBufferAsprintf(&buf, " <active>%d</active>\n", def->current); + virBufferAsprintf(&buf, "<active>%d</active>\n", def->current); + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</domainsnapshot>\n"); if (virBufferError(&buf)) { -- 1.8.5.3

On 03/06/2014 08:24 AM, Laine Stump wrote:
All leading spaces in domain snapshot xml format functions have been replaced with appropriate calls to virBufferAdjustIndent(). This will make it easier to call other similarly fixed format functions (e.g. domain device format functions). --- src/conf/snapshot_conf.c | 48 +++++++++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 21 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

This was very simple, since the only place that had hardcoded indentation was a few items in the network status xml. --- src/conf/network_conf.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index bac0465..1d5781e 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2806,17 +2806,17 @@ virNetworkObjFormat(virNetworkObjPtr net, goto no_memory; virBufferAddLit(&buf, "<networkstatus>\n"); - virBufferAsprintf(&buf, " <class_id bitmap='%s'/>\n", class_id); - virBufferAsprintf(&buf, " <floor sum='%llu'/>\n", net->floor_sum); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<class_id bitmap='%s'/>\n", class_id); + virBufferAsprintf(&buf, "<floor sum='%llu'/>\n", net->floor_sum); VIR_FREE(class_id); for (i = 0; i < VIR_NETWORK_TAINT_LAST; i++) { if (net->taint & (1 << i)) - virBufferAsprintf(&buf, " <taint flag='%s'/>\n", + virBufferAsprintf(&buf, "<taint flag='%s'/>\n", virNetworkTaintTypeToString(i)); } - virBufferAdjustIndent(&buf, 2); if (virNetworkDefFormatBuf(&buf, net->def, flags) < 0) goto error; -- 1.8.5.3

On 03/06/2014 08:24 AM, Laine Stump wrote:
This was very simple, since the only place that had hardcoded indentation was a few items in the network status xml. --- src/conf/network_conf.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

These format functions needed the ability to be indented by an arbitrary amount, but were written before the introduction of virBufferAdjustIndent(). They instead used the much more clunky method of adding a "level" arg to every format function, and padding with spaces using the "%*s" printf format specifier (giving it the level, and "", which has the effect of adding level spaces to the output). While eliminating the hardcoded indentation in other xml, I decided it was finally time to also modernize the interface formatter code to make it more consistent. --- src/conf/interface_conf.c | 137 ++++++++++++++++++++++------------------------ 1 file changed, 64 insertions(+), 73 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 8053307..65ab2fa 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -1,7 +1,7 @@ /* * interface_conf.c: interfaces XML handling * - * Copyright (C) 2006-2010, 2013 Red Hat, Inc. + * Copyright (C) 2006-2010, 2013, 2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -41,8 +41,7 @@ VIR_ENUM_IMPL(virInterface, static virInterfaceDefPtr virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType); static int -virInterfaceDefDevFormat(virBufferPtr buf, - const virInterfaceDef *def, int level); +virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def); static void virInterfaceIpDefFree(virInterfaceIpDefPtr def) { @@ -857,13 +856,12 @@ virInterfaceDefPtr virInterfaceDefParseFile(const char *filename) } static int -virInterfaceBridgeDefFormat(virBufferPtr buf, - const virInterfaceDef *def, int level) +virInterfaceBridgeDefFormat(virBufferPtr buf, const virInterfaceDef *def) { size_t i; int ret = 0; - virBufferAsprintf(buf, "%*s <bridge", level*2, ""); + virBufferAddLit(buf, "<bridge"); if (def->data.bridge.stp == 1) virBufferAddLit(buf, " stp='on'"); else if (def->data.bridge.stp == 0) @@ -871,25 +869,25 @@ virInterfaceBridgeDefFormat(virBufferPtr buf, if (def->data.bridge.delay != NULL) virBufferAsprintf(buf, " delay='%s'", def->data.bridge.delay); virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); for (i = 0; i < def->data.bridge.nbItf; i++) { - if (virInterfaceDefDevFormat(buf, - def->data.bridge.itf[i], level+2) < 0) + if (virInterfaceDefDevFormat(buf, def->data.bridge.itf[i]) < 0) ret = -1; } - virBufferAsprintf(buf, "%*s </bridge>\n", level*2, ""); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</bridge>\n"); return ret; } static int -virInterfaceBondDefFormat(virBufferPtr buf, - const virInterfaceDef *def, int level) +virInterfaceBondDefFormat(virBufferPtr buf, const virInterfaceDef *def) { size_t i; int ret = 0; - virBufferAsprintf(buf, "%*s <bond", level*2, ""); + virBufferAddLit(buf, "<bond"); if (def->data.bond.mode == VIR_INTERFACE_BOND_BALRR) virBufferAddLit(buf, " mode='balance-rr'"); else if (def->data.bond.mode == VIR_INTERFACE_BOND_ABACKUP) @@ -905,10 +903,11 @@ virInterfaceBondDefFormat(virBufferPtr buf, else if (def->data.bond.mode == VIR_INTERFACE_BOND_BALALB) virBufferAddLit(buf, " mode='balance-alb'"); virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); if (def->data.bond.monit == VIR_INTERFACE_BOND_MONIT_MII) { - virBufferAsprintf(buf, "%*s <miimon freq='%d'", - level*2, "", def->data.bond.frequency); + virBufferAsprintf(buf, "<miimon freq='%d'", + def->data.bond.frequency); if (def->data.bond.downdelay > 0) virBufferAsprintf(buf, " downdelay='%d'", def->data.bond.downdelay); if (def->data.bond.updelay > 0) @@ -924,8 +923,7 @@ virInterfaceBondDefFormat(virBufferPtr buf, "%s", _("bond arp monitoring has no target")); return -1; } - virBufferAsprintf(buf, "%*s <arpmon interval='%d' target='%s'", - level*2, "", + virBufferAsprintf(buf, "<arpmon interval='%d' target='%s'", def->data.bond.interval, def->data.bond.target); if (def->data.bond.validate == VIR_INTERFACE_BOND_ARP_ACTIVE) virBufferAddLit(buf, " validate='active'"); @@ -936,17 +934,17 @@ virInterfaceBondDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } for (i = 0; i < def->data.bond.nbItf; i++) { - if (virInterfaceDefDevFormat(buf, def->data.bond.itf[i], level+2) < 0) + if (virInterfaceDefDevFormat(buf, def->data.bond.itf[i]) < 0) ret = -1; } - virBufferAsprintf(buf, "%*s </bond>\n", level*2, ""); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</bond>\n"); return ret; } static int -virInterfaceVlanDefFormat(virBufferPtr buf, - const virInterfaceDef *def, int level) +virInterfaceVlanDefFormat(virBufferPtr buf, const virInterfaceDef *def) { if (def->data.vlan.tag == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -954,48 +952,45 @@ virInterfaceVlanDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, "%*s <vlan tag='%s'", - level*2, "", def->data.vlan.tag); + virBufferAsprintf(buf, "<vlan tag='%s'", def->data.vlan.tag); if (def->data.vlan.devname != NULL) { virBufferAddLit(buf, ">\n"); - virBufferAsprintf(buf, "%*s <interface name='%s'/>\n", - level*2, "", def->data.vlan.devname); - virBufferAsprintf(buf, "%*s </vlan>\n", level*2, ""); + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<interface name='%s'/>\n", + def->data.vlan.devname); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</vlan>\n"); } else virBufferAddLit(buf, "/>\n"); return 0; } static int -virInterfaceProtocolDefFormat(virBufferPtr buf, const virInterfaceDef *def, - int level) +virInterfaceProtocolDefFormat(virBufferPtr buf, const virInterfaceDef *def) { size_t i, j; for (i = 0; i < def->nprotos; i++) { - virBufferAsprintf(buf, "%*s <protocol family='%s'>\n", - level*2, "", def->protos[i]->family); - - if (def->protos[i]->autoconf) { - virBufferAsprintf(buf, "%*s <autoconf/>\n", level*2, ""); - } + virBufferAsprintf(buf, "<protocol family='%s'>\n", + def->protos[i]->family); + virBufferAdjustIndent(buf, 2); + if (def->protos[i]->autoconf) + virBufferAddLit(buf, "<autoconf/>\n"); if (def->protos[i]->dhcp) { if (def->protos[i]->peerdns == 0) - virBufferAsprintf(buf, "%*s <dhcp peerdns='no'/>\n", - level*2, ""); + virBufferAddLit(buf, "<dhcp peerdns='no'/>\n"); else if (def->protos[i]->peerdns == 1) - virBufferAsprintf(buf, "%*s <dhcp peerdns='yes'/>\n", - level*2, ""); + virBufferAddLit(buf, "<dhcp peerdns='yes'/>\n"); else - virBufferAsprintf(buf, "%*s <dhcp/>\n", level*2, ""); + virBufferAddLit(buf, "<dhcp/>\n"); } for (j = 0; j < def->protos[i]->nips; j++) { if (def->protos[i]->ips[j]->address != NULL) { - virBufferAsprintf(buf, "%*s <ip address='%s'", level*2, "", + virBufferAsprintf(buf, "<ip address='%s'", def->protos[i]->ips[j]->address); if (def->protos[i]->ips[j]->prefix != 0) { virBufferAsprintf(buf, " prefix='%d'", @@ -1005,19 +1000,20 @@ virInterfaceProtocolDefFormat(virBufferPtr buf, const virInterfaceDef *def, } } if (def->protos[i]->gateway != NULL) { - virBufferAsprintf(buf, "%*s <route gateway='%s'/>\n", - level*2, "", def->protos[i]->gateway); + virBufferAsprintf(buf, "<route gateway='%s'/>\n", + def->protos[i]->gateway); } - virBufferAsprintf(buf, "%*s </protocol>\n", level*2, ""); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</protocol>\n"); } return 0; } static int virInterfaceStartmodeDefFormat(virBufferPtr buf, - enum virInterfaceStartMode startmode, - int level) { + enum virInterfaceStartMode startmode) +{ const char *mode; switch (startmode) { case VIR_INTERFACE_START_UNSPECIFIED: @@ -1036,13 +1032,12 @@ virInterfaceStartmodeDefFormat(virBufferPtr buf, "%s", _("virInterfaceDefFormat unknown startmode")); return -1; } - virBufferAsprintf(buf, "%*s <start mode='%s'/>\n", level*2, "", mode); + virBufferAsprintf(buf, "<start mode='%s'/>\n", mode); return 0; } static int -virInterfaceDefDevFormat(virBufferPtr buf, - const virInterfaceDef *def, int level) +virInterfaceDefDevFormat(virBufferPtr buf, const virInterfaceDef *def) { const char *type = NULL; @@ -1064,52 +1059,48 @@ virInterfaceDefDevFormat(virBufferPtr buf, goto cleanup; } - virBufferAsprintf(buf, "%*s<interface type='%s' ", level*2, "", type); + virBufferAsprintf(buf, "<interface type='%s' ", type); if (def->name != NULL) virBufferEscapeString(buf, "name='%s'", def->name); virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); switch (def->type) { case VIR_INTERFACE_TYPE_ETHERNET: - virInterfaceStartmodeDefFormat(buf, def->startmode, level); + virInterfaceStartmodeDefFormat(buf, def->startmode); if (def->mac != NULL) - virBufferAsprintf(buf, "%*s <mac address='%s'/>\n", - level*2, "", def->mac); + virBufferAsprintf(buf, "<mac address='%s'/>\n", def->mac); if (def->mtu != 0) - virBufferAsprintf(buf, "%*s <mtu size='%d'/>\n", - level*2, "", def->mtu); - virInterfaceProtocolDefFormat(buf, def, level); + virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu); + virInterfaceProtocolDefFormat(buf, def); break; case VIR_INTERFACE_TYPE_BRIDGE: - virInterfaceStartmodeDefFormat(buf, def->startmode, level); + virInterfaceStartmodeDefFormat(buf, def->startmode); if (def->mtu != 0) - virBufferAsprintf(buf, "%*s <mtu size='%d'/>\n", - level*2, "", def->mtu); - virInterfaceProtocolDefFormat(buf, def, level); - virInterfaceBridgeDefFormat(buf, def, level); + virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu); + virInterfaceProtocolDefFormat(buf, def); + virInterfaceBridgeDefFormat(buf, def); break; case VIR_INTERFACE_TYPE_BOND: - virInterfaceStartmodeDefFormat(buf, def->startmode, level); + virInterfaceStartmodeDefFormat(buf, def->startmode); if (def->mtu != 0) - virBufferAsprintf(buf, "%*s <mtu size='%d'/>\n", - level*2, "", def->mtu); - virInterfaceProtocolDefFormat(buf, def, level); - virInterfaceBondDefFormat(buf, def, level); + virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu); + virInterfaceProtocolDefFormat(buf, def); + virInterfaceBondDefFormat(buf, def); break; case VIR_INTERFACE_TYPE_VLAN: - virInterfaceStartmodeDefFormat(buf, def->startmode, level); + virInterfaceStartmodeDefFormat(buf, def->startmode); if (def->mac != NULL) - virBufferAsprintf(buf, "%*s <mac address='%s'/>\n", - level*2, "", def->mac); + virBufferAsprintf(buf, "<mac address='%s'/>\n", def->mac); if (def->mtu != 0) - virBufferAsprintf(buf, "%*s <mtu size='%d'/>\n", - level*2, "", def->mtu); - virInterfaceProtocolDefFormat(buf, def, level); - virInterfaceVlanDefFormat(buf, def, level); + virBufferAsprintf(buf, "<mtu size='%d'/>\n", def->mtu); + virInterfaceProtocolDefFormat(buf, def); + virInterfaceVlanDefFormat(buf, def); break; } - virBufferAsprintf(buf, "%*s</interface>\n", level*2, ""); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</interface>\n"); if (virBufferError(buf)) goto no_memory; @@ -1124,7 +1115,7 @@ char *virInterfaceDefFormat(const virInterfaceDef *def) { virBuffer buf = VIR_BUFFER_INITIALIZER; - if (virInterfaceDefDevFormat(&buf, def, 0) < 0) { + if (virInterfaceDefDevFormat(&buf, def) < 0) { virBufferFreeAndReset(&buf); return NULL; } -- 1.8.5.3

On 03/06/2014 08:24 AM, Laine Stump wrote:
These format functions needed the ability to be indented by an arbitrary amount, but were written before the introduction of virBufferAdjustIndent(). They instead used the much more clunky method of adding a "level" arg to every format function, and padding with spaces using the "%*s" printf format specifier (giving it the level, and "", which has the effect of adding level spaces to the output).
While eliminating the hardcoded indentation in other xml, I decided it was finally time to also modernize the interface formatter code to make it more consistent. --- src/conf/interface_conf.c | 137 ++++++++++++++++++++++------------------------ 1 file changed, 64 insertions(+), 73 deletions(-)
About time!
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 8053307..65ab2fa 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -1,7 +1,7 @@ /* * interface_conf.c: interfaces XML handling * - * Copyright (C) 2006-2010, 2013 Red Hat, Inc. + * Copyright (C) 2006-2010, 2013, 2014 Red Hat, Inc.
Per git log, Red Hat people touched this file in 2011 and 2012, so you could simplify this to 2006-2014. ACK. Again, it's nice that 'make check' has tests of our generated output files, so that we cover most (if not all) of these changes to prove that the output is unchanged. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 03/06/2014 08:39 PM, Eric Blake wrote:
On 03/06/2014 08:24 AM, Laine Stump wrote:
These format functions needed the ability to be indented by an arbitrary amount, but were written before the introduction of virBufferAdjustIndent(). They instead used the much more clunky method of adding a "level" arg to every format function, and padding with spaces using the "%*s" printf format specifier (giving it the level, and "", which has the effect of adding level spaces to the output).
While eliminating the hardcoded indentation in other xml, I decided it was finally time to also modernize the interface formatter code to make it more consistent. --- src/conf/interface_conf.c | 137 ++++++++++++++++++++++------------------------ 1 file changed, 64 insertions(+), 73 deletions(-) About time!
You're telling me! It was me that originally put in the "%*s" usage, which seemed cool at the time, but looks really silly in comparison to having indentation built into virBuffer*.
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 8053307..65ab2fa 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -1,7 +1,7 @@ /* * interface_conf.c: interfaces XML handling * - * Copyright (C) 2006-2010, 2013 Red Hat, Inc. + * Copyright (C) 2006-2010, 2013, 2014 Red Hat, Inc. Per git log, Red Hat people touched this file in 2011 and 2012, so you could simplify this to 2006-2014.
Yeah, whenever I notice a comma-separated list of copyright dates on a Red Hat notice, I check the log for commits from @redhat.com addresses during the missing years, and merge them if possible.

This file was using multiple virBuffers, inserting the contents of buf3 into buf2, then inserting the contents of buf2 into buf1, rather than the more conventional method of just passing around a single virBufferPtr and streaming everything into that single buffer. This was unnecessary, and also made it more difficult to make indentation relative, because when you insert a string into a buffer, the indentation of the buffer is only applied once at the beginning of the string, *not* each time a newline is encountered in the string. --- src/conf/nwfilter_conf.c | 94 ++++++++++++++-------------------------------- src/conf/nwfilter_params.c | 6 ++- 2 files changed, 32 insertions(+), 68 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 52e1c06..588261c 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2,7 +2,7 @@ * nwfilter_conf.c: network filter XML processing * (derived from storage_conf.c) * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * Copyright (C) 2010-2011 IBM Corporation @@ -3215,7 +3215,7 @@ virNWFilterRuleDefDetailsFormat(virBufferPtr buf, enum virNWFilterEntryItemFlags flags = item->flags; if ((flags & NWFILTER_ENTRY_ITEM_FLAG_EXISTS)) { if (!typeShown) { - virBufferAsprintf(buf, " <%s", type); + virBufferAsprintf(buf, "<%s", type); typeShown = true; neverShown = false; } @@ -3326,95 +3326,59 @@ virNWFilterRuleDefDetailsFormat(virBufferPtr buf, if (neverShown) virBufferAsprintf(buf, - " <%s/>\n", type); + "<%s/>\n", type); err_exit: return; } -static char * -virNWFilterRuleDefFormat(virNWFilterRuleDefPtr def) +static int +virNWFilterRuleDefFormat(virBufferPtr buf, virNWFilterRuleDefPtr def) { size_t i; - virBuffer buf = VIR_BUFFER_INITIALIZER; - virBuffer buf2 = VIR_BUFFER_INITIALIZER; - char *data; + bool subelement = false; - virBufferAsprintf(&buf, " <rule action='%s' direction='%s' priority='%d'", + virBufferAsprintf(buf, "<rule action='%s' direction='%s' priority='%d'", virNWFilterRuleActionTypeToString(def->action), virNWFilterRuleDirectionTypeToString(def->tt), def->priority); if ((def->flags & RULE_FLAG_NO_STATEMATCH)) - virBufferAddLit(&buf, " statematch='false'"); + virBufferAddLit(buf, " statematch='false'"); + virBufferAdjustIndent(buf, 2); i = 0; while (virAttr[i].id) { if (virAttr[i].prtclType == def->prtclType) { - virNWFilterRuleDefDetailsFormat(&buf2, + if (!subelement) + virBufferAddLit(buf, ">\n"); + virNWFilterRuleDefDetailsFormat(buf, virAttr[i].id, virAttr[i].att, def); + subelement = true; break; } i++; } - if (virBufferError(&buf2)) - goto no_memory; - - data = virBufferContentAndReset(&buf2); - - if (data) { - virBufferAddLit(&buf, ">\n"); - virBufferAsprintf(&buf, "%s </rule>\n", data); - VIR_FREE(data); - } else - virBufferAddLit(&buf, "/>\n"); - - if (virBufferError(&buf)) - goto no_memory; - - return virBufferContentAndReset(&buf); - -no_memory: - virReportOOMError(); - virBufferFreeAndReset(&buf); - virBufferFreeAndReset(&buf2); - - return NULL; -} - - -static char * -virNWFilterIncludeDefFormat(virNWFilterIncludeDefPtr inc) -{ - virBuffer buf = VIR_BUFFER_INITIALIZER; - - virBufferAdjustIndent(&buf, 2); - if (virNWFilterFormatParamAttributes(&buf, inc->params, - inc->filterref) < 0) { - virBufferFreeAndReset(&buf); - return NULL; - } - virBufferAdjustIndent(&buf, -2); - if (virBufferError(&buf)) { - virReportOOMError(); - virBufferFreeAndReset(&buf); - return NULL; - } - - return virBufferContentAndReset(&buf); + virBufferAdjustIndent(buf, -2); + if (subelement) + virBufferAddLit(buf, "</rule>\n"); + else + virBufferAddLit(buf, "/>\n"); + return 0; } -static char * -virNWFilterEntryFormat(virNWFilterEntryPtr entry) +static int +virNWFilterEntryFormat(virBufferPtr buf, virNWFilterEntryPtr entry) { if (entry->rule) - return virNWFilterRuleDefFormat(entry->rule); - return virNWFilterIncludeDefFormat(entry->include); + return virNWFilterRuleDefFormat(buf, entry->rule); + return virNWFilterFormatParamAttributes(buf, entry->include->params, + entry->include->filterref); } @@ -3424,7 +3388,6 @@ virNWFilterDefFormat(const virNWFilterDef *def) virBuffer buf = VIR_BUFFER_INITIALIZER; char uuid[VIR_UUID_STRING_BUFLEN]; size_t i; - char *xml; virBufferAsprintf(&buf, "<filter name='%s' chain='%s'", def->name, @@ -3433,18 +3396,17 @@ virNWFilterDefFormat(const virNWFilterDef *def) virBufferAsprintf(&buf, " priority='%d'", def->chainPriority); virBufferAddLit(&buf, ">\n"); + virBufferAdjustIndent(&buf, 2); virUUIDFormat(def->uuid, uuid); - virBufferAsprintf(&buf, " <uuid>%s</uuid>\n", uuid); + virBufferAsprintf(&buf, "<uuid>%s</uuid>\n", uuid); for (i = 0; i < def->nentries; i++) { - xml = virNWFilterEntryFormat(def->filterEntries[i]); - if (!xml) + if (virNWFilterEntryFormat(&buf, def->filterEntries[i]) < 0) goto err_exit; - virBufferAdd(&buf, xml, -1); - VIR_FREE(xml); } + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</filter>\n"); if (virBufferError(&buf)) diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c index 589fcf8..d189157 100644 --- a/src/conf/nwfilter_params.c +++ b/src/conf/nwfilter_params.c @@ -1,7 +1,7 @@ /* * nwfilter_params.c: parsing and data maintenance of filter parameters * - * Copyright (C) 2011-2013 Red Hat, Inc. + * Copyright (C) 2011-2014 Red Hat, Inc. * Copyright (C) 2010 IBM Corporation * * This library is free software; you can redistribute it and/or @@ -907,6 +907,7 @@ virNWFilterFormatParamAttributes(virBufferPtr buf, virBufferAsprintf(buf, "<filterref filter='%s'", filterref); if (numKeys) { virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); for (i = 0; i < numKeys; i++) { const virNWFilterVarValue *value = items[i].value; @@ -914,11 +915,12 @@ virNWFilterFormatParamAttributes(virBufferPtr buf, for (j = 0; j < card; j++) virBufferAsprintf(buf, - " <parameter name='%s' value='%s'/>\n", + "<parameter name='%s' value='%s'/>\n", (const char *)items[i].key, virNWFilterVarValueGetNthValue(value, j)); } + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</filterref>\n"); } else { virBufferAddLit(buf, "/>\n"); -- 1.8.5.3

On 03/06/2014 08:24 AM, Laine Stump wrote:
This file was using multiple virBuffers, inserting the contents of buf3 into buf2, then inserting the contents of buf2 into buf1, rather than the more conventional method of just passing around a single virBufferPtr and streaming everything into that single buffer. This was unnecessary, and also made it more difficult to make indentation relative, because when you insert a string into a buffer, the indentation of the buffer is only applied once at the beginning of the string, *not* each time a newline is encountered in the string. --- src/conf/nwfilter_conf.c | 94 ++++++++++++++-------------------------------- src/conf/nwfilter_params.c | 6 ++- 2 files changed, 32 insertions(+), 68 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

There were a lot of changes here, but all very mechanical. For some reason, the virBufferPtr had been named "xml" instead of "buf" in this file, so since the indentation changing touched almost every line using the buffer, I took this chance to change its name for "buf" for consistency with every other file. --- src/conf/capabilities.c | 183 +++++++++++++++++++++++++++--------------------- 1 file changed, 103 insertions(+), 80 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index c1c4ab8..3022b45 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -1,7 +1,7 @@ /* * capabilities.c: hypervisor capabilities * - * Copyright (C) 2006-2008, 2010-2011, 2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -745,7 +745,7 @@ virCapabilitiesDefaultGuestEmulator(virCapsPtr caps, } static int -virCapabilitiesFormatNUMATopology(virBufferPtr xml, +virCapabilitiesFormatNUMATopology(virBufferPtr buf, size_t ncells, virCapsHostNUMACellPtr *cells) { @@ -753,21 +753,23 @@ virCapabilitiesFormatNUMATopology(virBufferPtr xml, size_t j; char *siblings; - virBufferAddLit(xml, " <topology>\n"); - virBufferAsprintf(xml, " <cells num='%zu'>\n", ncells); + virBufferAddLit(buf, "<topology>\n"); + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<cells num='%zu'>\n", ncells); + virBufferAdjustIndent(buf, 2); for (i = 0; i < ncells; i++) { - virBufferAsprintf(xml, " <cell id='%d'>\n", cells[i]->num); + virBufferAsprintf(buf, "<cell id='%d'>\n", cells[i]->num); + virBufferAdjustIndent(buf, 2); /* Print out the numacell memory total if it is available */ if (cells[i]->mem) - virBufferAsprintf(xml, - " <memory unit='KiB'>%llu</memory>\n", + virBufferAsprintf(buf, "<memory unit='KiB'>%llu</memory>\n", cells[i]->mem); - virBufferAsprintf(xml, " <cpus num='%d'>\n", cells[i]->ncpus); + virBufferAsprintf(buf, "<cpus num='%d'>\n", cells[i]->ncpus); + virBufferAdjustIndent(buf, 2); for (j = 0; j < cells[i]->ncpus; j++) { - virBufferAsprintf(xml, " <cpu id='%d'", - cells[i]->cpus[j].id); + virBufferAsprintf(buf, "<cpu id='%d'", cells[i]->cpus[j].id); if (cells[i]->cpus[j].siblings) { if (!(siblings = virBitmapFormat(cells[i]->cpus[j].siblings))) { @@ -775,22 +777,24 @@ virCapabilitiesFormatNUMATopology(virBufferPtr xml, return -1; } - virBufferAsprintf(xml, + virBufferAsprintf(buf, " socket_id='%d' core_id='%d' siblings='%s'", cells[i]->cpus[j].socket_id, cells[i]->cpus[j].core_id, siblings); VIR_FREE(siblings); } - virBufferAddLit(xml, "/>\n"); + virBufferAddLit(buf, "/>\n"); } - - virBufferAddLit(xml, " </cpus>\n"); - virBufferAddLit(xml, " </cell>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</cpus>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</cell>\n"); } - virBufferAddLit(xml, " </cells>\n"); - virBufferAddLit(xml, " </topology>\n"); - + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</cells>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</topology>\n"); return 0; } @@ -805,142 +809,160 @@ virCapabilitiesFormatNUMATopology(virBufferPtr xml, char * virCapabilitiesFormatXML(virCapsPtr caps) { - virBuffer xml = VIR_BUFFER_INITIALIZER; + virBuffer buf = VIR_BUFFER_INITIALIZER; size_t i, j, k; char host_uuid[VIR_UUID_STRING_BUFLEN]; - virBufferAddLit(&xml, "<capabilities>\n\n"); - virBufferAddLit(&xml, " <host>\n"); + virBufferAddLit(&buf, "<capabilities>\n\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAddLit(&buf, "<host>\n"); + virBufferAdjustIndent(&buf, 2); if (virUUIDIsValid(caps->host.host_uuid)) { virUUIDFormat(caps->host.host_uuid, host_uuid); - virBufferAsprintf(&xml, " <uuid>%s</uuid>\n", host_uuid); + virBufferAsprintf(&buf, "<uuid>%s</uuid>\n", host_uuid); } - virBufferAddLit(&xml, " <cpu>\n"); + virBufferAddLit(&buf, "<cpu>\n"); + virBufferAdjustIndent(&buf, 2); + if (caps->host.arch) - virBufferAsprintf(&xml, " <arch>%s</arch>\n", + virBufferAsprintf(&buf, "<arch>%s</arch>\n", virArchToString(caps->host.arch)); - if (caps->host.nfeatures) { - virBufferAddLit(&xml, " <features>\n"); + virBufferAddLit(&buf, "<features>\n"); + virBufferAdjustIndent(&buf, 2); for (i = 0; i < caps->host.nfeatures; i++) { - virBufferAsprintf(&xml, " <%s/>\n", + virBufferAsprintf(&buf, "<%s/>\n", caps->host.features[i]); } - virBufferAddLit(&xml, " </features>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</features>\n"); } + virCPUDefFormatBuf(&buf, caps->host.cpu, 0); - virBufferAdjustIndent(&xml, 6); - virCPUDefFormatBuf(&xml, caps->host.cpu, 0); - virBufferAdjustIndent(&xml, -6); - - virBufferAddLit(&xml, " </cpu>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</cpu>\n"); /* The PM query was successful. */ if (caps->host.powerMgmt) { /* The host supports some PM features. */ unsigned int pm = caps->host.powerMgmt; - virBufferAddLit(&xml, " <power_management>\n"); + virBufferAddLit(&buf, "<power_management>\n"); + virBufferAdjustIndent(&buf, 2); while (pm) { int bit = ffs(pm) - 1; - virBufferAsprintf(&xml, " <%s/>\n", + virBufferAsprintf(&buf, "<%s/>\n", virCapsHostPMTargetTypeToString(bit)); pm &= ~(1U << bit); } - virBufferAddLit(&xml, " </power_management>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</power_management>\n"); } else { /* The host does not support any PM feature. */ - virBufferAddLit(&xml, " <power_management/>\n"); + virBufferAddLit(&buf, "<power_management/>\n"); } if (caps->host.offlineMigrate) { - virBufferAddLit(&xml, " <migration_features>\n"); + virBufferAddLit(&buf, "<migration_features>\n"); + virBufferAdjustIndent(&buf, 2); if (caps->host.liveMigrate) - virBufferAddLit(&xml, " <live/>\n"); + virBufferAddLit(&buf, "<live/>\n"); if (caps->host.nmigrateTrans) { - virBufferAddLit(&xml, " <uri_transports>\n"); + virBufferAddLit(&buf, "<uri_transports>\n"); + virBufferAdjustIndent(&buf, 2); for (i = 0; i < caps->host.nmigrateTrans; i++) { - virBufferAsprintf(&xml, " <uri_transport>%s</uri_transport>\n", + virBufferAsprintf(&buf, "<uri_transport>%s</uri_transport>\n", caps->host.migrateTrans[i]); } - virBufferAddLit(&xml, " </uri_transports>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</uri_transports>\n"); } - virBufferAddLit(&xml, " </migration_features>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</migration_features>\n"); } if (caps->host.nnumaCell && - virCapabilitiesFormatNUMATopology(&xml, caps->host.nnumaCell, + virCapabilitiesFormatNUMATopology(&buf, caps->host.nnumaCell, caps->host.numaCell) < 0) return NULL; for (i = 0; i < caps->host.nsecModels; i++) { - virBufferAddLit(&xml, " <secmodel>\n"); - virBufferAsprintf(&xml, " <model>%s</model>\n", + virBufferAddLit(&buf, "<secmodel>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<model>%s</model>\n", caps->host.secModels[i].model); - virBufferAsprintf(&xml, " <doi>%s</doi>\n", + virBufferAsprintf(&buf, "<doi>%s</doi>\n", caps->host.secModels[i].doi); for (j = 0; j < caps->host.secModels[i].nlabels; j++) { - virBufferAsprintf(&xml, " <baselabel type='%s'>%s</baselabel>\n", + virBufferAsprintf(&buf, "<baselabel type='%s'>%s</baselabel>\n", caps->host.secModels[i].labels[j].type, caps->host.secModels[i].labels[j].label); } - virBufferAddLit(&xml, " </secmodel>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</secmodel>\n"); } - virBufferAddLit(&xml, " </host>\n\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</host>\n\n"); for (i = 0; i < caps->nguests; i++) { - virBufferAddLit(&xml, " <guest>\n"); - virBufferAsprintf(&xml, " <os_type>%s</os_type>\n", + virBufferAddLit(&buf, "<guest>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<os_type>%s</os_type>\n", caps->guests[i]->ostype); if (caps->guests[i]->arch.id) - virBufferAsprintf(&xml, " <arch name='%s'>\n", + virBufferAsprintf(&buf, "<arch name='%s'>\n", virArchToString(caps->guests[i]->arch.id)); - virBufferAsprintf(&xml, " <wordsize>%d</wordsize>\n", + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<wordsize>%d</wordsize>\n", caps->guests[i]->arch.wordsize); if (caps->guests[i]->arch.defaultInfo.emulator) - virBufferAsprintf(&xml, " <emulator>%s</emulator>\n", + virBufferAsprintf(&buf, "<emulator>%s</emulator>\n", caps->guests[i]->arch.defaultInfo.emulator); if (caps->guests[i]->arch.defaultInfo.loader) - virBufferAsprintf(&xml, " <loader>%s</loader>\n", + virBufferAsprintf(&buf, "<loader>%s</loader>\n", caps->guests[i]->arch.defaultInfo.loader); for (j = 0; j < caps->guests[i]->arch.defaultInfo.nmachines; j++) { virCapsGuestMachinePtr machine = caps->guests[i]->arch.defaultInfo.machines[j]; - virBufferAddLit(&xml, " <machine"); + virBufferAddLit(&buf, "<machine"); if (machine->canonical) - virBufferAsprintf(&xml, " canonical='%s'", machine->canonical); + virBufferAsprintf(&buf, " canonical='%s'", machine->canonical); if (machine->maxCpus > 0) - virBufferAsprintf(&xml, " maxCpus='%d'", machine->maxCpus); - virBufferAsprintf(&xml, ">%s</machine>\n", machine->name); + virBufferAsprintf(&buf, " maxCpus='%d'", machine->maxCpus); + virBufferAsprintf(&buf, ">%s</machine>\n", machine->name); } for (j = 0; j < caps->guests[i]->arch.ndomains; j++) { - virBufferAsprintf(&xml, " <domain type='%s'>\n", + virBufferAsprintf(&buf, "<domain type='%s'>\n", caps->guests[i]->arch.domains[j]->type); + virBufferAdjustIndent(&buf, 2); if (caps->guests[i]->arch.domains[j]->info.emulator) - virBufferAsprintf(&xml, " <emulator>%s</emulator>\n", + virBufferAsprintf(&buf, "<emulator>%s</emulator>\n", caps->guests[i]->arch.domains[j]->info.emulator); if (caps->guests[i]->arch.domains[j]->info.loader) - virBufferAsprintf(&xml, " <loader>%s</loader>\n", + virBufferAsprintf(&buf, "<loader>%s</loader>\n", caps->guests[i]->arch.domains[j]->info.loader); for (k = 0; k < caps->guests[i]->arch.domains[j]->info.nmachines; k++) { virCapsGuestMachinePtr machine = caps->guests[i]->arch.domains[j]->info.machines[k]; - virBufferAddLit(&xml, " <machine"); + virBufferAddLit(&buf, "<machine"); if (machine->canonical) - virBufferAsprintf(&xml, " canonical='%s'", machine->canonical); + virBufferAsprintf(&buf, " canonical='%s'", machine->canonical); if (machine->maxCpus > 0) - virBufferAsprintf(&xml, " maxCpus='%d'", machine->maxCpus); - virBufferAsprintf(&xml, ">%s</machine>\n", machine->name); + virBufferAsprintf(&buf, " maxCpus='%d'", machine->maxCpus); + virBufferAsprintf(&buf, ">%s</machine>\n", machine->name); } - virBufferAddLit(&xml, " </domain>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</domain>\n"); } - virBufferAddLit(&xml, " </arch>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</arch>\n"); if (caps->guests[i]->nfeatures) { - virBufferAddLit(&xml, " <features>\n"); + virBufferAddLit(&buf, "<features>\n"); + virBufferAdjustIndent(&buf, 2); for (j = 0; j < caps->guests[i]->nfeatures; j++) { if (STREQ(caps->guests[i]->features[j]->name, "pae") || @@ -948,30 +970,31 @@ virCapabilitiesFormatXML(virCapsPtr caps) STREQ(caps->guests[i]->features[j]->name, "ia64_be") || STREQ(caps->guests[i]->features[j]->name, "cpuselection") || STREQ(caps->guests[i]->features[j]->name, "deviceboot")) { - virBufferAsprintf(&xml, " <%s/>\n", + virBufferAsprintf(&buf, "<%s/>\n", caps->guests[i]->features[j]->name); } else { - virBufferAsprintf(&xml, " <%s default='%s' toggle='%s'/>\n", + virBufferAsprintf(&buf, "<%s default='%s' toggle='%s'/>\n", caps->guests[i]->features[j]->name, caps->guests[i]->features[j]->defaultOn ? "on" : "off", caps->guests[i]->features[j]->toggle ? "yes" : "no"); } } - virBufferAddLit(&xml, " </features>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</features>\n"); } - - virBufferAddLit(&xml, " </guest>\n\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</guest>\n\n"); } + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</capabilities>\n"); - virBufferAddLit(&xml, "</capabilities>\n"); - - if (virBufferError(&xml)) { - virBufferFreeAndReset(&xml); + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); return NULL; } - return virBufferContentAndReset(&xml); + return virBufferContentAndReset(&buf); } /* get the maximum ID of cpus in the host */ -- 1.8.5.3

On 03/06/2014 08:24 AM, Laine Stump wrote:
There were a lot of changes here, but all very mechanical. For some reason, the virBufferPtr had been named "xml" instead of "buf" in this file, so since the indentation changing touched almost every line using the buffer, I took this chance to change its name for "buf" for consistency with every other file. --- src/conf/capabilities.c | 183 +++++++++++++++++++++++++++--------------------- 1 file changed, 103 insertions(+), 80 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Completely mechanical changes, but there were a lot of lines so I made it a separate patch. --- src/conf/node_device_conf.c | 207 ++++++++++++++++++++++++-------------------- 1 file changed, 112 insertions(+), 95 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index ea85cff..65ee4b3 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1,7 +1,7 @@ /* * node_device_conf.c: config handling for node devices * - * Copyright (C) 2009-2013 Red Hat, Inc. + * Copyright (C) 2009-2014 Red Hat, Inc. * Copyright (C) 2008 Virtual Iron Software, Inc. * Copyright (C) 2008 David F. Lively * @@ -235,70 +235,77 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDef *def) size_t i = 0; virBufferAddLit(&buf, "<device>\n"); - virBufferEscapeString(&buf, " <name>%s</name>\n", def->name); - virBufferEscapeString(&buf, " <path>%s</path>\n", def->sysfs_path); - if (def->parent) { - virBufferEscapeString(&buf, " <parent>%s</parent>\n", def->parent); - } + virBufferAdjustIndent(&buf, 2); + virBufferEscapeString(&buf, "<name>%s</name>\n", def->name); + virBufferEscapeString(&buf, "<path>%s</path>\n", def->sysfs_path); + if (def->parent) + virBufferEscapeString(&buf, "<parent>%s</parent>\n", def->parent); if (def->driver) { - virBufferAddLit(&buf, " <driver>\n"); - virBufferEscapeString(&buf, " <name>%s</name>\n", def->driver); - virBufferAddLit(&buf, " </driver>\n"); + virBufferAddLit(&buf, "<driver>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferEscapeString(&buf, "<name>%s</name>\n", def->driver); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</driver>\n"); } for (caps = def->caps; caps; caps = caps->next) { char uuidstr[VIR_UUID_STRING_BUFLEN]; union _virNodeDevCapData *data = &caps->data; - virBufferAsprintf(&buf, " <capability type='%s'>\n", + virBufferAsprintf(&buf, "<capability type='%s'>\n", virNodeDevCapTypeToString(caps->type)); + virBufferAdjustIndent(&buf, 2); switch (caps->type) { case VIR_NODE_DEV_CAP_SYSTEM: if (data->system.product_name) - virBufferEscapeString(&buf, " <product>%s</product>\n", + virBufferEscapeString(&buf, "<product>%s</product>\n", data->system.product_name); - virBufferAddLit(&buf, " <hardware>\n"); + virBufferAddLit(&buf, "<hardware>\n"); + virBufferAdjustIndent(&buf, 2); if (data->system.hardware.vendor_name) - virBufferEscapeString(&buf, " <vendor>%s</vendor>\n", + virBufferEscapeString(&buf, "<vendor>%s</vendor>\n", data->system.hardware.vendor_name); if (data->system.hardware.version) - virBufferEscapeString(&buf, " <version>%s</version>\n", + virBufferEscapeString(&buf, "<version>%s</version>\n", data->system.hardware.version); if (data->system.hardware.serial) - virBufferEscapeString(&buf, " <serial>%s</serial>\n", + virBufferEscapeString(&buf, "<serial>%s</serial>\n", data->system.hardware.serial); virUUIDFormat(data->system.hardware.uuid, uuidstr); - virBufferAsprintf(&buf, " <uuid>%s</uuid>\n", uuidstr); - virBufferAddLit(&buf, " </hardware>\n"); - virBufferAddLit(&buf, " <firmware>\n"); + virBufferAsprintf(&buf, "<uuid>%s</uuid>\n", uuidstr); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</hardware>\n"); + + virBufferAddLit(&buf, "<firmware>\n"); + virBufferAdjustIndent(&buf, 2); if (data->system.firmware.vendor_name) - virBufferEscapeString(&buf, " <vendor>%s</vendor>\n", + virBufferEscapeString(&buf, "<vendor>%s</vendor>\n", data->system.firmware.vendor_name); if (data->system.firmware.version) - virBufferEscapeString(&buf, " <version>%s</version>\n", + virBufferEscapeString(&buf, "<version>%s</version>\n", data->system.firmware.version); if (data->system.firmware.release_date) - virBufferEscapeString(&buf, - " <release_date>%s</release_date>\n", + virBufferEscapeString(&buf, "<release_date>%s</release_date>\n", data->system.firmware.release_date); - virBufferAddLit(&buf, " </firmware>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</firmware>\n"); break; case VIR_NODE_DEV_CAP_PCI_DEV: - virBufferAsprintf(&buf, " <domain>%d</domain>\n", + virBufferAsprintf(&buf, "<domain>%d</domain>\n", data->pci_dev.domain); - virBufferAsprintf(&buf, " <bus>%d</bus>\n", data->pci_dev.bus); - virBufferAsprintf(&buf, " <slot>%d</slot>\n", + virBufferAsprintf(&buf, "<bus>%d</bus>\n", data->pci_dev.bus); + virBufferAsprintf(&buf, "<slot>%d</slot>\n", data->pci_dev.slot); - virBufferAsprintf(&buf, " <function>%d</function>\n", + virBufferAsprintf(&buf, "<function>%d</function>\n", data->pci_dev.function); - virBufferAsprintf(&buf, " <product id='0x%04x'", + virBufferAsprintf(&buf, "<product id='0x%04x'", data->pci_dev.product); if (data->pci_dev.product_name) virBufferEscapeString(&buf, ">%s</product>\n", data->pci_dev.product_name); else virBufferAddLit(&buf, " />\n"); - virBufferAsprintf(&buf, " <vendor id='0x%04x'", + virBufferAsprintf(&buf, "<vendor id='0x%04x'", data->pci_dev.vendor); if (data->pci_dev.vendor_name) virBufferEscapeString(&buf, ">%s</vendor>\n", @@ -306,56 +313,62 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDef *def) else virBufferAddLit(&buf, " />\n"); if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION) { - virBufferAddLit(&buf, " <capability type='phys_function'>\n"); + virBufferAddLit(&buf, "<capability type='phys_function'>\n"); + virBufferAdjustIndent(&buf, 2); virBufferAsprintf(&buf, - " <address domain='0x%.4x' bus='0x%.2x' " + "<address domain='0x%.4x' bus='0x%.2x' " "slot='0x%.2x' function='0x%.1x'/>\n", data->pci_dev.physical_function->domain, data->pci_dev.physical_function->bus, data->pci_dev.physical_function->slot, data->pci_dev.physical_function->function); - virBufferAddLit(&buf, " </capability>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</capability>\n"); } if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION) { - virBufferAddLit(&buf, " <capability type='virt_functions'>\n"); + virBufferAddLit(&buf, "<capability type='virt_functions'>\n"); + virBufferAdjustIndent(&buf, 2); for (i = 0; i < data->pci_dev.num_virtual_functions; i++) { virBufferAsprintf(&buf, - " <address domain='0x%.4x' bus='0x%.2x' " + "<address domain='0x%.4x' bus='0x%.2x' " "slot='0x%.2x' function='0x%.1x'/>\n", data->pci_dev.virtual_functions[i]->domain, data->pci_dev.virtual_functions[i]->bus, data->pci_dev.virtual_functions[i]->slot, data->pci_dev.virtual_functions[i]->function); } - virBufferAddLit(&buf, " </capability>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</capability>\n"); } if (data->pci_dev.nIommuGroupDevices) { - virBufferAsprintf(&buf, " <iommuGroup number='%d'>\n", + virBufferAsprintf(&buf, "<iommuGroup number='%d'>\n", data->pci_dev.iommuGroupNumber); + virBufferAdjustIndent(&buf, 2); for (i = 0; i < data->pci_dev.nIommuGroupDevices; i++) { virBufferAsprintf(&buf, - " <address domain='0x%.4x' bus='0x%.2x' " + "<address domain='0x%.4x' bus='0x%.2x' " "slot='0x%.2x' function='0x%.1x'/>\n", data->pci_dev.iommuGroupDevices[i]->domain, data->pci_dev.iommuGroupDevices[i]->bus, data->pci_dev.iommuGroupDevices[i]->slot, data->pci_dev.iommuGroupDevices[i]->function); } - virBufferAddLit(&buf, " </iommuGroup>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</iommuGroup>\n"); } break; case VIR_NODE_DEV_CAP_USB_DEV: - virBufferAsprintf(&buf, " <bus>%d</bus>\n", data->usb_dev.bus); - virBufferAsprintf(&buf, " <device>%d</device>\n", + virBufferAsprintf(&buf, "<bus>%d</bus>\n", data->usb_dev.bus); + virBufferAsprintf(&buf, "<device>%d</device>\n", data->usb_dev.device); - virBufferAsprintf(&buf, " <product id='0x%04x'", + virBufferAsprintf(&buf, "<product id='0x%04x'", data->usb_dev.product); if (data->usb_dev.product_name) virBufferEscapeString(&buf, ">%s</product>\n", data->usb_dev.product_name); else virBufferAddLit(&buf, " />\n"); - virBufferAsprintf(&buf, " <vendor id='0x%04x'", + virBufferAsprintf(&buf, "<vendor id='0x%04x'", data->usb_dev.vendor); if (data->usb_dev.vendor_name) virBufferEscapeString(&buf, ">%s</vendor>\n", @@ -364,130 +377,132 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDef *def) virBufferAddLit(&buf, " />\n"); break; case VIR_NODE_DEV_CAP_USB_INTERFACE: - virBufferAsprintf(&buf, " <number>%d</number>\n", + virBufferAsprintf(&buf, "<number>%d</number>\n", data->usb_if.number); - virBufferAsprintf(&buf, " <class>%d</class>\n", + virBufferAsprintf(&buf, "<class>%d</class>\n", data->usb_if._class); - virBufferAsprintf(&buf, " <subclass>%d</subclass>\n", + virBufferAsprintf(&buf, "<subclass>%d</subclass>\n", data->usb_if.subclass); - virBufferAsprintf(&buf, " <protocol>%d</protocol>\n", + virBufferAsprintf(&buf, "<protocol>%d</protocol>\n", data->usb_if.protocol); if (data->usb_if.description) virBufferEscapeString(&buf, - " <description>%s</description>\n", + "<description>%s</description>\n", data->usb_if.description); break; case VIR_NODE_DEV_CAP_NET: - virBufferEscapeString(&buf, " <interface>%s</interface>\n", + virBufferEscapeString(&buf, "<interface>%s</interface>\n", data->net.ifname); if (data->net.address) - virBufferEscapeString(&buf, " <address>%s</address>\n", + virBufferEscapeString(&buf, "<address>%s</address>\n", data->net.address); if (data->net.subtype != VIR_NODE_DEV_CAP_NET_LAST) { const char *subtyp = virNodeDevNetCapTypeToString(data->net.subtype); - virBufferEscapeString(&buf, " <capability type='%s'/>\n", + virBufferEscapeString(&buf, "<capability type='%s'/>\n", subtyp); } break; case VIR_NODE_DEV_CAP_SCSI_HOST: - virBufferAsprintf(&buf, " <host>%d</host>\n", + virBufferAsprintf(&buf, "<host>%d</host>\n", data->scsi_host.host); if (data->scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { - virBufferAddLit(&buf, " <capability type='fc_host'>\n"); - virBufferEscapeString(&buf, " <wwnn>%s</wwnn>\n", + virBufferAddLit(&buf, "<capability type='fc_host'>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferEscapeString(&buf, "<wwnn>%s</wwnn>\n", data->scsi_host.wwnn); - virBufferEscapeString(&buf, " <wwpn>%s</wwpn>\n", + virBufferEscapeString(&buf, "<wwpn>%s</wwpn>\n", data->scsi_host.wwpn); - virBufferEscapeString(&buf, " <fabric_wwn>%s</fabric_wwn>\n", + virBufferEscapeString(&buf, "<fabric_wwn>%s</fabric_wwn>\n", data->scsi_host.fabric_wwn); - virBufferAddLit(&buf, " </capability>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</capability>\n"); } if (data->scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS) { - virBufferAddLit(&buf, " <capability type='vport_ops'>\n"); - virBufferAsprintf(&buf, " <max_vports>%d</max_vports>\n", + virBufferAddLit(&buf, "<capability type='vport_ops'>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<max_vports>%d</max_vports>\n", data->scsi_host.max_vports); - virBufferAsprintf(&buf, " <vports>%d</vports>\n", + virBufferAsprintf(&buf, "<vports>%d</vports>\n", data->scsi_host.vports); - virBufferAddLit(&buf, " </capability>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</capability>\n"); } break; case VIR_NODE_DEV_CAP_SCSI_TARGET: - virBufferEscapeString(&buf, " <target>%s</target>\n", + virBufferEscapeString(&buf, "<target>%s</target>\n", data->scsi_target.name); break; case VIR_NODE_DEV_CAP_SCSI: - virBufferAsprintf(&buf, " <host>%d</host>\n", data->scsi.host); - virBufferAsprintf(&buf, " <bus>%d</bus>\n", data->scsi.bus); - virBufferAsprintf(&buf, " <target>%d</target>\n", + virBufferAsprintf(&buf, "<host>%d</host>\n", data->scsi.host); + virBufferAsprintf(&buf, "<bus>%d</bus>\n", data->scsi.bus); + virBufferAsprintf(&buf, "<target>%d</target>\n", data->scsi.target); - virBufferAsprintf(&buf, " <lun>%d</lun>\n", data->scsi.lun); + virBufferAsprintf(&buf, "<lun>%d</lun>\n", data->scsi.lun); if (data->scsi.type) - virBufferEscapeString(&buf, " <type>%s</type>\n", + virBufferEscapeString(&buf, "<type>%s</type>\n", data->scsi.type); break; case VIR_NODE_DEV_CAP_STORAGE: - virBufferEscapeString(&buf, " <block>%s</block>\n", - data->storage.block); + virBufferEscapeString(&buf, "<block>%s</block>\n", + data->storage.block); if (data->storage.bus) - virBufferEscapeString(&buf, " <bus>%s</bus>\n", - data->storage.bus); + virBufferEscapeString(&buf, "<bus>%s</bus>\n", + data->storage.bus); if (data->storage.drive_type) - virBufferEscapeString(&buf, " <drive_type>%s</drive_type>\n", - data->storage.drive_type); + virBufferEscapeString(&buf, "<drive_type>%s</drive_type>\n", + data->storage.drive_type); if (data->storage.model) - virBufferEscapeString(&buf, " <model>%s</model>\n", - data->storage.model); + virBufferEscapeString(&buf, "<model>%s</model>\n", + data->storage.model); if (data->storage.vendor) - virBufferEscapeString(&buf, " <vendor>%s</vendor>\n", - data->storage.vendor); + virBufferEscapeString(&buf, "<vendor>%s</vendor>\n", + data->storage.vendor); if (data->storage.serial) - virBufferAsprintf(&buf, " <serial>%s</serial>\n", + virBufferAsprintf(&buf, "<serial>%s</serial>\n", data->storage.serial); if (data->storage.flags & VIR_NODE_DEV_CAP_STORAGE_REMOVABLE) { int avl = data->storage.flags & VIR_NODE_DEV_CAP_STORAGE_REMOVABLE_MEDIA_AVAILABLE; - virBufferAddLit(&buf, " <capability type='removable'>\n"); - virBufferAsprintf(&buf, - " <media_available>%d" + virBufferAddLit(&buf, "<capability type='removable'>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<media_available>%d" "</media_available>\n", avl ? 1 : 0); - virBufferAsprintf(&buf, " <media_size>%llu</media_size>\n", + virBufferAsprintf(&buf, "<media_size>%llu</media_size>\n", data->storage.removable_media_size); if (data->storage.media_label) virBufferEscapeString(&buf, - " <media_label>%s</media_label>\n", - data->storage.media_label); - + "<media_label>%s</media_label>\n", + data->storage.media_label); if (data->storage.logical_block_size > 0) - virBufferAsprintf(&buf, " <logical_block_size>%llu" + virBufferAsprintf(&buf, "<logical_block_size>%llu" "</logical_block_size>\n", data->storage.logical_block_size); if (data->storage.num_blocks > 0) virBufferAsprintf(&buf, - " <num_blocks>%llu</num_blocks>\n", + "<num_blocks>%llu</num_blocks>\n", data->storage.num_blocks); - virBufferAddLit(&buf, " </capability>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</capability>\n"); } else { - virBufferAsprintf(&buf, " <size>%llu</size>\n", + virBufferAsprintf(&buf, "<size>%llu</size>\n", data->storage.size); if (data->storage.logical_block_size > 0) - virBufferAsprintf(&buf, " <logical_block_size>%llu" + virBufferAsprintf(&buf, "<logical_block_size>%llu" "</logical_block_size>\n", data->storage.logical_block_size); if (data->storage.num_blocks > 0) - virBufferAsprintf(&buf, - " <num_blocks>%llu</num_blocks>\n", + virBufferAsprintf(&buf, "<num_blocks>%llu</num_blocks>\n", data->storage.num_blocks); } if (data->storage.flags & VIR_NODE_DEV_CAP_STORAGE_HOTPLUGGABLE) - virBufferAddLit(&buf, - " <capability type='hotpluggable' />\n"); + virBufferAddLit(&buf, "<capability type='hotpluggable' />\n"); break; case VIR_NODE_DEV_CAP_SCSI_GENERIC: - virBufferEscapeString(&buf, " <char>%s</char>\n", + virBufferEscapeString(&buf, "<char>%s</char>\n", data->sg.path); break; case VIR_NODE_DEV_CAP_FC_HOST: @@ -497,9 +512,11 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDef *def) break; } - virBufferAddLit(&buf, " </capability>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</capability>\n"); } + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</device>\n"); if (virBufferError(&buf)) -- 1.8.5.3

On 03/06/2014 08:24 AM, Laine Stump wrote:
Completely mechanical changes, but there were a lot of lines so I made it a separate patch. --- src/conf/node_device_conf.c | 207 ++++++++++++++++++++++++-------------------- 1 file changed, 112 insertions(+), 95 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Again completely mechanical, but a lot of lines. --- src/conf/storage_conf.c | 174 +++++++++++++++++++++---------------- src/conf/storage_encryption_conf.c | 6 +- 2 files changed, 105 insertions(+), 75 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index e4232e9..2f195b6 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1,7 +1,7 @@ /* * storage_conf.c: config handling for storage driver * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1074,10 +1074,12 @@ virStoragePoolSourceFormat(virBufferPtr buf, size_t i, j; char uuid[VIR_UUID_STRING_BUFLEN]; - virBufferAddLit(buf, " <source>\n"); + virBufferAddLit(buf, "<source>\n"); + virBufferAdjustIndent(buf, 2); + if ((options->flags & VIR_STORAGE_POOL_SOURCE_HOST) && src->nhost) { for (i = 0; i < src->nhost; i++) { - virBufferEscapeString(buf, " <host name='%s'", + virBufferEscapeString(buf, "<host name='%s'", src->hosts[i].name); if (src->hosts[i].port) virBufferAsprintf(buf, " port='%d'", src->hosts[i].port); @@ -1089,28 +1091,30 @@ virStoragePoolSourceFormat(virBufferPtr buf, src->ndevice) { for (i = 0; i < src->ndevice; i++) { if (src->devices[i].nfreeExtent) { - virBufferEscapeString(buf, " <device path='%s'>\n", + virBufferEscapeString(buf, "<device path='%s'>\n", src->devices[i].path); + virBufferAdjustIndent(buf, 2); for (j = 0; j < src->devices[i].nfreeExtent; j++) { - virBufferAsprintf(buf, " <freeExtent start='%llu' end='%llu'/>\n", + virBufferAsprintf(buf, "<freeExtent start='%llu' end='%llu'/>\n", src->devices[i].freeExtents[j].start, src->devices[i].freeExtents[j].end); } - virBufferAddLit(buf, " </device>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</device>\n"); } else { - virBufferEscapeString(buf, " <device path='%s'/>\n", + virBufferEscapeString(buf, "<device path='%s'/>\n", src->devices[i].path); } } } if (options->flags & VIR_STORAGE_POOL_SOURCE_DIR) - virBufferEscapeString(buf, " <dir path='%s'/>\n", src->dir); + virBufferEscapeString(buf, "<dir path='%s'/>\n", src->dir); if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER)) { if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST || src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) - virBufferAsprintf(buf, " <adapter type='%s'", + virBufferAsprintf(buf, "<adapter type='%s'", virStoragePoolSourceAdapterTypeTypeToString(src->adapter.type)); if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { @@ -1126,14 +1130,16 @@ virStoragePoolSourceFormat(virBufferPtr buf, } if (options->flags & VIR_STORAGE_POOL_SOURCE_NAME) - virBufferEscapeString(buf, " <name>%s</name>\n", src->name); + virBufferEscapeString(buf, "<name>%s</name>\n", src->name); if ((options->flags & VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN) && src->initiator.iqn) { - virBufferAddLit(buf, " <initiator>\n"); - virBufferEscapeString(buf, " <iqn name='%s'/>\n", + virBufferAddLit(buf, "<initiator>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<iqn name='%s'/>\n", src->initiator.iqn); - virBufferAddLit(buf, " </initiator>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</initiator>\n"); } if (options->formatToString) { @@ -1144,19 +1150,20 @@ virStoragePoolSourceFormat(virBufferPtr buf, src->format); return -1; } - virBufferAsprintf(buf, " <format type='%s'/>\n", format); + virBufferAsprintf(buf, "<format type='%s'/>\n", format); } if (src->authType == VIR_STORAGE_POOL_AUTH_CHAP || src->authType == VIR_STORAGE_POOL_AUTH_CEPHX) { - virBufferAsprintf(buf, " <auth type='%s' ", + virBufferAsprintf(buf, "<auth type='%s' ", virStoragePoolAuthTypeTypeToString(src->authType)); virBufferEscapeString(buf, "username='%s'>\n", (src->authType == VIR_STORAGE_POOL_AUTH_CHAP ? src->auth.chap.username : src->auth.cephx.username)); + virBufferAdjustIndent(buf, 2); - virBufferAddLit(buf, " <secret"); + virBufferAddLit(buf, "<secret"); if (src->auth.cephx.secret.uuidUsable) { virUUIDFormat(src->auth.cephx.secret.uuid, uuid); virBufferAsprintf(buf, " uuid='%s'", uuid); @@ -1167,14 +1174,15 @@ virStoragePoolSourceFormat(virBufferPtr buf, } virBufferAddLit(buf, "/>\n"); - virBufferAddLit(buf, " </auth>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</auth>\n"); } - virBufferEscapeString(buf, " <vendor name='%s'/>\n", src->vendor); - virBufferEscapeString(buf, " <product name='%s'/>\n", src->product); - - virBufferAddLit(buf, " </source>\n"); + virBufferEscapeString(buf, "<vendor name='%s'/>\n", src->vendor); + virBufferEscapeString(buf, "<product name='%s'/>\n", src->product); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</source>\n"); return 0; } @@ -1198,16 +1206,17 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def) goto cleanup; } virBufferAsprintf(&buf, "<pool type='%s'>\n", type); - virBufferEscapeString(&buf, " <name>%s</name>\n", def->name); + virBufferAdjustIndent(&buf, 2); + virBufferEscapeString(&buf, "<name>%s</name>\n", def->name); virUUIDFormat(def->uuid, uuid); - virBufferAsprintf(&buf, " <uuid>%s</uuid>\n", uuid); + virBufferAsprintf(&buf, "<uuid>%s</uuid>\n", uuid); - virBufferAsprintf(&buf, " <capacity unit='bytes'>%llu</capacity>\n", + virBufferAsprintf(&buf, "<capacity unit='bytes'>%llu</capacity>\n", def->capacity); - virBufferAsprintf(&buf, " <allocation unit='bytes'>%llu</allocation>\n", + virBufferAsprintf(&buf, "<allocation unit='bytes'>%llu</allocation>\n", def->allocation); - virBufferAsprintf(&buf, " <available unit='bytes'>%llu</available>\n", + virBufferAsprintf(&buf, "<available unit='bytes'>%llu</available>\n", def->available); if (virStoragePoolSourceFormat(&buf, options, &def->source) < 0) @@ -1218,24 +1227,29 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def) if (def->type != VIR_STORAGE_POOL_RBD && def->type != VIR_STORAGE_POOL_SHEEPDOG && def->type != VIR_STORAGE_POOL_GLUSTER) { - virBufferAddLit(&buf, " <target>\n"); + virBufferAddLit(&buf, "<target>\n"); + virBufferAdjustIndent(&buf, 2); - virBufferEscapeString(&buf, " <path>%s</path>\n", def->target.path); + virBufferEscapeString(&buf, "<path>%s</path>\n", def->target.path); - virBufferAddLit(&buf, " <permissions>\n"); - virBufferAsprintf(&buf, " <mode>0%o</mode>\n", + virBufferAddLit(&buf, "<permissions>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<mode>0%o</mode>\n", def->target.perms.mode); - virBufferAsprintf(&buf, " <owner>%d</owner>\n", + virBufferAsprintf(&buf, "<owner>%d</owner>\n", (int) def->target.perms.uid); - virBufferAsprintf(&buf, " <group>%d</group>\n", + virBufferAsprintf(&buf, "<group>%d</group>\n", (int) def->target.perms.gid); - virBufferEscapeString(&buf, " <label>%s</label>\n", + virBufferEscapeString(&buf, "<label>%s</label>\n", def->target.perms.label); - virBufferAddLit(&buf, " </permissions>\n"); - virBufferAddLit(&buf, " </target>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</permissions>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</target>\n"); } + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</pool>\n"); if (virBufferError(&buf)) @@ -1501,7 +1515,7 @@ virStorageVolTimestampFormat(virBufferPtr buf, const char *name, { if (ts->tv_nsec < 0) return; - virBufferAsprintf(buf, " <%s>%llu", name, + virBufferAsprintf(buf, "<%s>%llu", name, (unsigned long long) ts->tv_sec); if (ts->tv_nsec) virBufferAsprintf(buf, ".%09ld", ts->tv_nsec); @@ -1514,9 +1528,10 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, virStorageVolTargetPtr def, const char *type) { - virBufferAsprintf(buf, " <%s>\n", type); + virBufferAsprintf(buf, "<%s>\n", type); + virBufferAdjustIndent(buf, 2); - virBufferEscapeString(buf, " <path>%s</path>\n", def->path); + virBufferEscapeString(buf, "<path>%s</path>\n", def->path); if (options->formatToString) { const char *format = (options->formatToString)(def->format); @@ -1526,63 +1541,69 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, def->format); return -1; } - virBufferAsprintf(buf, " <format type='%s'/>\n", format); + virBufferAsprintf(buf, "<format type='%s'/>\n", format); } - virBufferAddLit(buf, " <permissions>\n"); - virBufferAsprintf(buf, " <mode>0%o</mode>\n", + virBufferAddLit(buf, "<permissions>\n"); + virBufferAdjustIndent(buf, 2); + + virBufferAsprintf(buf, "<mode>0%o</mode>\n", def->perms.mode); - virBufferAsprintf(buf, " <owner>%u</owner>\n", + virBufferAsprintf(buf, "<owner>%u</owner>\n", (unsigned int) def->perms.uid); - virBufferAsprintf(buf, " <group>%u</group>\n", + virBufferAsprintf(buf, "<group>%u</group>\n", (unsigned int) def->perms.gid); - virBufferEscapeString(buf, " <label>%s</label>\n", + virBufferEscapeString(buf, "<label>%s</label>\n", def->perms.label); - virBufferAddLit(buf, " </permissions>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</permissions>\n"); if (def->timestamps) { - virBufferAddLit(buf, " <timestamps>\n"); + virBufferAddLit(buf, "<timestamps>\n"); + virBufferAdjustIndent(buf, 2); virStorageVolTimestampFormat(buf, "atime", &def->timestamps->atime); virStorageVolTimestampFormat(buf, "mtime", &def->timestamps->mtime); virStorageVolTimestampFormat(buf, "ctime", &def->timestamps->ctime); virStorageVolTimestampFormat(buf, "btime", &def->timestamps->btime); - virBufferAddLit(buf, " </timestamps>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</timestamps>\n"); } - if (def->encryption) { - virBufferAdjustIndent(buf, 4); - if (virStorageEncryptionFormat(buf, def->encryption) < 0) + if (def->encryption && + virStorageEncryptionFormat(buf, def->encryption) < 0) return -1; - virBufferAdjustIndent(buf, -4); - } - virBufferEscapeString(buf, " <compat>%s</compat>\n", def->compat); + virBufferEscapeString(buf, "<compat>%s</compat>\n", def->compat); if (options->featureToString && def->features) { size_t i; bool b; bool empty = virBitmapIsAllClear(def->features); - if (empty) - virBufferAddLit(buf, " <features/>\n"); - else - virBufferAddLit(buf, " <features>\n"); + if (empty) { + virBufferAddLit(buf, "<features/>\n"); + } else { + virBufferAddLit(buf, "<features>\n"); + virBufferAdjustIndent(buf, 2); + } for (i = 0; i < VIR_STORAGE_FILE_FEATURE_LAST; i++) { ignore_value(virBitmapGetBit(def->features, i, &b)); if (b) - virBufferAsprintf(buf, " <%s/>\n", + virBufferAsprintf(buf, "<%s/>\n", options->featureToString(i)); } - if (!empty) - virBufferAddLit(buf, " </features>\n"); + if (!empty) { + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</features>\n"); + } } - virBufferAsprintf(buf, " </%s>\n", type); - + virBufferAdjustIndent(buf, -2); + virBufferAsprintf(buf, "</%s>\n", type); return 0; } @@ -1599,9 +1620,12 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool, virBufferAsprintf(&buf, "<volume type='%s'>\n", virStorageVolTypeToString(def->type)); - virBufferEscapeString(&buf, " <name>%s</name>\n", def->name); - virBufferEscapeString(&buf, " <key>%s</key>\n", def->key); - virBufferAddLit(&buf, " <source>\n"); + virBufferAdjustIndent(&buf, 2); + + virBufferEscapeString(&buf, "<name>%s</name>\n", def->name); + virBufferEscapeString(&buf, "<key>%s</key>\n", def->key); + virBufferAddLit(&buf, "<source>\n"); + virBufferAdjustIndent(&buf, 2); if (def->source.nextent) { size_t i; @@ -1610,26 +1634,29 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool, if (thispath == NULL || STRNEQ(thispath, def->source.extents[i].path)) { if (thispath != NULL) - virBufferAddLit(&buf, " </device>\n"); + virBufferAddLit(&buf, "</device>\n"); - virBufferEscapeString(&buf, " <device path='%s'>\n", + virBufferEscapeString(&buf, "<device path='%s'>\n", def->source.extents[i].path); } - virBufferAsprintf(&buf, - " <extent start='%llu' end='%llu'/>\n", + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<extent start='%llu' end='%llu'/>\n", def->source.extents[i].start, def->source.extents[i].end); + virBufferAdjustIndent(&buf, -2); thispath = def->source.extents[i].path; } if (thispath != NULL) - virBufferAddLit(&buf, " </device>\n"); + virBufferAddLit(&buf, "</device>\n"); } - virBufferAddLit(&buf, " </source>\n"); - virBufferAsprintf(&buf, " <capacity unit='bytes'>%llu</capacity>\n", + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</source>\n"); + + virBufferAsprintf(&buf, "<capacity unit='bytes'>%llu</capacity>\n", def->capacity); - virBufferAsprintf(&buf, " <allocation unit='bytes'>%llu</allocation>\n", + virBufferAsprintf(&buf, "<allocation unit='bytes'>%llu</allocation>\n", def->allocation); if (virStorageVolTargetDefFormat(options, &buf, @@ -1641,6 +1668,7 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool, &def->backingStore, "backingStore") < 0) goto cleanup; + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</volume>\n"); if (virBufferError(&buf)) diff --git a/src/conf/storage_encryption_conf.c b/src/conf/storage_encryption_conf.c index c2fafe3..798541c 100644 --- a/src/conf/storage_encryption_conf.c +++ b/src/conf/storage_encryption_conf.c @@ -1,7 +1,7 @@ /* * storage_encryption_conf.c: volume encryption information * - * Copyright (C) 2009-2011 Red Hat, Inc. + * Copyright (C) 2009-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -218,7 +218,7 @@ virStorageEncryptionSecretFormat(virBufferPtr buf, } virUUIDFormat(secret->uuid, uuidstr); - virBufferAsprintf(buf, " <secret type='%s' uuid='%s'/>\n", + virBufferAsprintf(buf, "<secret type='%s' uuid='%s'/>\n", type, uuidstr); return 0; } @@ -237,12 +237,14 @@ virStorageEncryptionFormat(virBufferPtr buf, return -1; } virBufferAsprintf(buf, "<encryption format='%s'>\n", format); + virBufferAdjustIndent(buf, 2); for (i = 0; i < enc->nsecrets; i++) { if (virStorageEncryptionSecretFormat(buf, enc->secrets[i]) < 0) return -1; } + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</encryption>\n"); return 0; -- 1.8.5.3

On 03/06/2014 08:24 AM, Laine Stump wrote:
Again completely mechanical, but a lot of lines. --- src/conf/storage_conf.c | 174 +++++++++++++++++++++---------------- src/conf/storage_encryption_conf.c | 6 +- 2 files changed, 105 insertions(+), 75 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

These last files had such a small change count I just put them into a single patch. --- src/conf/cpu_conf.c | 11 ++++++----- src/conf/netdev_bandwidth_conf.c | 6 ++++-- src/conf/netdev_vlan_conf.c | 6 ++++-- src/conf/netdev_vport_profile_conf.c | 6 ++++-- src/conf/secret_conf.c | 20 ++++++++++++-------- 5 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 3d015f2..4b982c9 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -1,7 +1,7 @@ /* * cpu_conf.c: CPU XML handling * - * Copyright (C) 2009-2013 Red Hat, Inc. + * Copyright (C) 2009-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -541,12 +541,11 @@ virCPUDefFormatBufFull(virBufferPtr buf, } } virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); if (def->arch) - virBufferAsprintf(buf, " <arch>%s</arch>\n", + virBufferAsprintf(buf, "<arch>%s</arch>\n", virArchToString(def->arch)); - - virBufferAdjustIndent(buf, 2); if (virCPUDefFormatBuf(buf, def, flags) < 0) return -1; virBufferAdjustIndent(buf, -2); @@ -645,12 +644,14 @@ virCPUDefFormatBuf(virBufferPtr buf, if (def->ncells) { virBufferAddLit(buf, "<numa>\n"); + virBufferAdjustIndent(buf, 2); for (i = 0; i < def->ncells; i++) { - virBufferAddLit(buf, " <cell"); + virBufferAddLit(buf, "<cell"); virBufferAsprintf(buf, " cpus='%s'", def->cells[i].cpustr); virBufferAsprintf(buf, " memory='%d'", def->cells[i].mem); virBufferAddLit(buf, "/>\n"); } + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</numa>\n"); } return 0; diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c index da14909..ed02704 100644 --- a/src/conf/netdev_bandwidth_conf.c +++ b/src/conf/netdev_bandwidth_conf.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2009-2011 Red Hat, Inc. + * Copyright (C) 2009-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -214,7 +214,7 @@ virNetDevBandwidthRateFormat(virNetDevBandwidthRatePtr def, return 0; if (def->average || def->floor) { - virBufferAsprintf(buf, " <%s", elem_name); + virBufferAsprintf(buf, "<%s", elem_name); if (def->average) virBufferAsprintf(buf, " average='%llu'", def->average); @@ -257,9 +257,11 @@ virNetDevBandwidthFormat(virNetDevBandwidthPtr def, virBufferPtr buf) } virBufferAddLit(buf, "<bandwidth>\n"); + virBufferAdjustIndent(buf, 2); if (virNetDevBandwidthRateFormat(def->in, buf, "inbound") < 0 || virNetDevBandwidthRateFormat(def->out, buf, "outbound") < 0) goto cleanup; + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</bandwidth>\n"); ret = 0; diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c index dbe203e..e5196d5 100644 --- a/src/conf/netdev_vlan_conf.c +++ b/src/conf/netdev_vlan_conf.c @@ -154,6 +154,7 @@ virNetDevVlanFormat(const virNetDevVlan *def, virBufferPtr buf) } virBufferAsprintf(buf, "<vlan%s>\n", def->trunk ? " trunk='yes'" : ""); + virBufferAdjustIndent(buf, 2); for (i = 0; i < def->nTags; i++) { if (def->nativeMode != VIR_NATIVE_VLAN_MODE_DEFAULT && def->nativeTag == def->tag[i]) { @@ -163,12 +164,13 @@ virNetDevVlanFormat(const virNetDevVlan *def, virBufferPtr buf) virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Bad value for nativeMode")); } - virBufferAsprintf(buf, " <tag id='%u' nativeMode='%s'/>\n", + virBufferAsprintf(buf, "<tag id='%u' nativeMode='%s'/>\n", def->tag[i], mode); } else { - virBufferAsprintf(buf, " <tag id='%u'/>\n", def->tag[i]); + virBufferAsprintf(buf, "<tag id='%u'/>\n", def->tag[i]); } } + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</vlan>\n"); return 0; } diff --git a/src/conf/netdev_vport_profile_conf.c b/src/conf/netdev_vport_profile_conf.c index 8d403c4..c7d2ab1 100644 --- a/src/conf/netdev_vport_profile_conf.c +++ b/src/conf/netdev_vport_profile_conf.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2009-2012 Red Hat, Inc. + * Copyright (C) 2009-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -232,7 +232,8 @@ virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort, virNetDevVPortTypeToString(type)); } } - virBufferAddLit(buf, " <parameters"); + virBufferAdjustIndent(buf, 2); + virBufferAddLit(buf, "<parameters"); if (virtPort->managerID_specified && (type == VIR_NETDEV_VPORT_PROFILE_8021QBG || @@ -274,6 +275,7 @@ virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort, } virBufferAddLit(buf, "/>\n"); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</virtualport>\n"); return 0; } diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index c3252d1..6fdefd0 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -1,7 +1,7 @@ /* * secret_conf.c: internal <secret> XML handling * - * Copyright (C) 2009, 2011, 2013 Red Hat, Inc. + * Copyright (C) 2009, 2011, 2013, 2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -254,27 +254,28 @@ virSecretDefFormatUsage(virBufferPtr buf, def->usage_type); return -1; } - virBufferAsprintf(buf, " <usage type='%s'>\n", type); + virBufferAsprintf(buf, "<usage type='%s'>\n", type); + virBufferAdjustIndent(buf, 2); switch (def->usage_type) { case VIR_SECRET_USAGE_TYPE_NONE: break; case VIR_SECRET_USAGE_TYPE_VOLUME: if (def->usage.volume != NULL) - virBufferEscapeString(buf, " <volume>%s</volume>\n", + virBufferEscapeString(buf, "<volume>%s</volume>\n", def->usage.volume); break; case VIR_SECRET_USAGE_TYPE_CEPH: if (def->usage.ceph != NULL) { - virBufferEscapeString(buf, " <name>%s</name>\n", + virBufferEscapeString(buf, "<name>%s</name>\n", def->usage.ceph); } break; case VIR_SECRET_USAGE_TYPE_ISCSI: if (def->usage.target != NULL) { - virBufferEscapeString(buf, " <target>%s</target>\n", + virBufferEscapeString(buf, "<target>%s</target>\n", def->usage.target); } break; @@ -285,7 +286,8 @@ virSecretDefFormatUsage(virBufferPtr buf, def->usage_type); return -1; } - virBufferAddLit(buf, " </usage>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</usage>\n"); return 0; } @@ -303,13 +305,15 @@ virSecretDefFormat(const virSecretDef *def) uuid = def->uuid; virUUIDFormat(uuid, uuidstr); - virBufferEscapeString(&buf, " <uuid>%s</uuid>\n", uuidstr); + virBufferAdjustIndent(&buf, 2); + virBufferEscapeString(&buf, "<uuid>%s</uuid>\n", uuidstr); if (def->description != NULL) - virBufferEscapeString(&buf, " <description>%s</description>\n", + virBufferEscapeString(&buf, "<description>%s</description>\n", def->description); if (def->usage_type != VIR_SECRET_USAGE_TYPE_NONE && virSecretDefFormatUsage(&buf, def) < 0) goto error; + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</secret>\n"); if (virBufferError(&buf)) -- 1.8.5.3

On 03/06/2014 08:24 AM, Laine Stump wrote:
These last files had such a small change count I just put them into a single patch. --- src/conf/cpu_conf.c | 11 ++++++----- src/conf/netdev_bandwidth_conf.c | 6 ++++-- src/conf/netdev_vlan_conf.c | 6 ++++-- src/conf/netdev_vport_profile_conf.c | 6 ++++-- src/conf/secret_conf.c | 20 ++++++++++++-------- 5 files changed, 30 insertions(+), 19 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 03/06/2014 08:24 AM, Laine Stump wrote: [0/9 in the subject line]
Many of the domain xml format functions (including all of the device format functions) had hard-coded spaces, which made for incorrect indentation when those functions were called in a different context (for example, commit 2122cf39 added <interface> XML into the document provided to a network hook script, and in this case it should have been indented by 2 spaces, but was instead indented by 6 spaces).
In that patch I mentioned doing a followup patch to make the device xml formatters more consistent. After doing that patch, it felt incomplete to not give the same treatment to the entire directory.
The one downside to this series is that it may create merge conflicts during backports, but fortunately the conflicts should all be fairly easy to resolve.
Missing from this series: qemuDomainObjPrivateXMLFormat in qemu_domain.c qemuMigrationCookieGraphicsXMLFormat in qemu_migration.c probably others. Also, it would be a good idea to add a syntax check to cfg.mk. I'd suggest a rule something like: sc_forbid_manual_xml_indent: @prohibit='virBuffer.*" +<' \ halt='use virBufferIndent when indenting xml' \ $(_sc_search_regexp) As I mentioned in the real 1/9, using local indentation is more compact than using virBufferIndent, but makes it harder to enforce - so if we go with your patches, a syntax check rule that enforces things will make it so we don't slip into old habits. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 03/06/2014 10:32 PM, Eric Blake wrote:
On 03/06/2014 08:24 AM, Laine Stump wrote:
[0/9 in the subject line]
Many of the domain xml format functions (including all of the device format functions) had hard-coded spaces, which made for incorrect indentation when those functions were called in a different context (for example, commit 2122cf39 added <interface> XML into the document provided to a network hook script, and in this case it should have been indented by 2 spaces, but was instead indented by 6 spaces).
In that patch I mentioned doing a followup patch to make the device xml formatters more consistent. After doing that patch, it felt incomplete to not give the same treatment to the entire directory.
The one downside to this series is that it may create merge conflicts during backports, but fortunately the conflicts should all be fairly easy to resolve. Missing from this series:
qemuDomainObjPrivateXMLFormat in qemu_domain.c qemuMigrationCookieGraphicsXMLFormat in qemu_migration.c
Ah, I didn't think to look in directories other than conf.
probably others. Also, it would be a good idea to add a syntax check to cfg.mk. I'd suggest a rule something like:
sc_forbid_manual_xml_indent: @prohibit='virBuffer.*" +<' \ halt='use virBufferIndent when indenting xml' \ $(_sc_search_regexp)
... and adding in that rule reveals a few more files that still have spaces. Sounds like something for me to do while I'm sitting in airports tomorrow :-)
As I mentioned in the real 1/9, using local indentation is more compact than using virBufferIndent, but makes it harder to enforce - so if we go with your patches, a syntax check rule that enforces things will make it so we don't slip into old habits.
I also had idle thoughts of doing away with virBufferAdjustIndent entirely, and adding an "xml pretty print" option to virBufferContentsAndReset, but that's probably too inefficient in comparison to what it does now. :-)
participants (2)
-
Eric Blake
-
Laine Stump