[libvirt] [PATCH 0/6] eliminate hardcoded indent from all XML - PART 2
This is a followup to https://www.redhat.com/archives/libvir-list/2014-March/msg00350.html Eric had ACKed all of those patches, but pointed out some omissions. The first patch of this series should be squashed into 1/9 of the previous series (which I haven't yet pushed), and the others are new. The final patch adds a syntax-check rule to keep new hardcoded spacing from creeping in. (I'm still pondering the idea of giving virBuffer a "pretty print" mode. It would certainly make it more difficult to mess up indentation). Laine Stump (6): squash this into domain conf XML "eliminate hardcoded indent" patch virsh: eliminate hardcoded indentation in xml generated for commands qemu: eliminate hardcoded indent from migration cookie xml util: eliminate hardcoded indent in virConnectSysInfo formatting qemu: elmininate hardcoded indent in capabilities cache XML build: detect/prohibit hardcoded XML indent in syntax-check cfg.mk | 7 ++++- src/conf/domain_conf.c | 2 +- src/lxc/lxc_domain.c | 4 +-- src/qemu/qemu_capabilities.c | 20 ++++++------ src/qemu/qemu_domain.c | 45 ++++++++++++++++----------- src/qemu/qemu_migration.c | 49 ++++++++++++++++++------------ src/util/virsysinfo.c | 72 ++++++++++++++++++++++++-------------------- tools/virsh-domain.c | 43 ++++++++++++++------------ tools/virsh-pool.c | 40 +++++++++++++++--------- tools/virsh-snapshot.c | 24 +++++++++------ tools/virsh-volume.c | 26 ++++++++++------ 11 files changed, 196 insertions(+), 136 deletions(-) -- 1.8.5.3
This should be squashed into the first patch of the previous "eliminate hardcoded indent" series (which was ACKed but hasn't yet been pushed): https://www.redhat.com/archives/libvir-list/2014-March/msg00361.html The initial patch neglected to change the functions that format private metadata in the qemu and lxc domain xml. I have verified that make check still passes when these changes are squashed into the original. --- src/conf/domain_conf.c | 2 +- src/lxc/lxc_domain.c | 4 ++-- src/qemu/qemu_domain.c | 45 +++++++++++++++++++++++++++------------------ 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c2d3c3d..8c4d64f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17681,13 +17681,13 @@ virDomainDefFormatInternal(virDomainDefPtr def, for (n = 0; n < def->nseclabels; n++) virSecurityLabelDefFormat(buf, def->seclabels[n], flags); - virBufferAdjustIndent(buf, -2); if (def->namespaceData && def->ns.format) { if ((def->ns.format)(buf, def->namespaceData) < 0) goto error; } + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</domain>\n"); if (virBufferError(buf)) diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index 83c5a4e..fe64a36 100644 --- a/src/lxc/lxc_domain.c +++ b/src/lxc/lxc_domain.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright IBM Corp. 2008 * * lxc_domain.h: LXC domain helpers @@ -53,7 +53,7 @@ static int virLXCDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) { virLXCDomainObjPrivatePtr priv = data; - virBufferAsprintf(buf, " <init pid='%llu'/>\n", + virBufferAsprintf(buf, "<init pid='%llu'/>\n", (unsigned long long)priv->initpid); return 0; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c321eda..bc0b8f7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1,7 +1,7 @@ /* * qemu_domain.c: QEMU domain private state * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -287,7 +287,7 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) break; } - virBufferEscapeString(buf, " <monitor path='%s'", monitorpath); + virBufferEscapeString(buf, "<monitor path='%s'", monitorpath); if (priv->monJSON) virBufferAddLit(buf, " json='1'"); virBufferAsprintf(buf, " type='%s'/>\n", @@ -297,34 +297,38 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) if (priv->nvcpupids) { size_t i; - virBufferAddLit(buf, " <vcpus>\n"); + virBufferAddLit(buf, "<vcpus>\n"); + virBufferAdjustIndent(buf, 2); for (i = 0; i < priv->nvcpupids; i++) { - virBufferAsprintf(buf, " <vcpu pid='%d'/>\n", priv->vcpupids[i]); + virBufferAsprintf(buf, "<vcpu pid='%d'/>\n", priv->vcpupids[i]); } - virBufferAddLit(buf, " </vcpus>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</vcpus>\n"); } if (priv->qemuCaps) { size_t i; - virBufferAddLit(buf, " <qemuCaps>\n"); + virBufferAddLit(buf, "<qemuCaps>\n"); + virBufferAdjustIndent(buf, 2); for (i = 0; i < QEMU_CAPS_LAST; i++) { if (virQEMUCapsGet(priv->qemuCaps, i)) { - virBufferAsprintf(buf, " <flag name='%s'/>\n", + virBufferAsprintf(buf, "<flag name='%s'/>\n", virQEMUCapsTypeToString(i)); } } - virBufferAddLit(buf, " </qemuCaps>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</qemuCaps>\n"); } if (priv->lockState) - virBufferAsprintf(buf, " <lockstate>%s</lockstate>\n", priv->lockState); + virBufferAsprintf(buf, "<lockstate>%s</lockstate>\n", priv->lockState); job = priv->job.active; if (!qemuDomainTrackJob(job)) priv->job.active = QEMU_JOB_NONE; if (priv->job.active || priv->job.asyncJob) { - virBufferAsprintf(buf, " <job type='%s' async='%s'", + virBufferAsprintf(buf, "<job type='%s' async='%s'", qemuDomainJobTypeToString(priv->job.active), qemuDomainAsyncJobTypeToString(priv->job.asyncJob)); if (priv->job.phase) { @@ -337,16 +341,18 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) priv->job.active = job; if (priv->fakeReboot) - virBufferAddLit(buf, " <fakereboot/>\n"); + virBufferAddLit(buf, "<fakereboot/>\n"); if (priv->qemuDevices && *priv->qemuDevices) { char **tmp = priv->qemuDevices; - virBufferAddLit(buf, " <devices>\n"); + virBufferAddLit(buf, "<devices>\n"); + virBufferAdjustIndent(buf, 2); while (*tmp) { - virBufferAsprintf(buf, " <device alias='%s'/>\n", *tmp); + virBufferAsprintf(buf, "<device alias='%s'/>\n", *tmp); tmp++; } - virBufferAddLit(buf, " </devices>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</devices>\n"); } return 0; @@ -651,18 +657,21 @@ qemuDomainDefNamespaceFormatXML(virBufferPtr buf, if (!cmd->num_args && !cmd->num_env) return 0; - virBufferAddLit(buf, " <qemu:commandline>\n"); + virBufferAddLit(buf, "<qemu:commandline>\n"); + virBufferAdjustIndent(buf, 2); + for (i = 0; i < cmd->num_args; i++) - virBufferEscapeString(buf, " <qemu:arg value='%s'/>\n", + virBufferEscapeString(buf, "<qemu:arg value='%s'/>\n", cmd->args[i]); for (i = 0; i < cmd->num_env; i++) { - virBufferAsprintf(buf, " <qemu:env name='%s'", cmd->env_name[i]); + virBufferAsprintf(buf, "<qemu:env name='%s'", cmd->env_name[i]); if (cmd->env_value[i]) virBufferEscapeString(buf, " value='%s'", cmd->env_value[i]); virBufferAddLit(buf, "/>\n"); } - virBufferAddLit(buf, " </qemu:commandline>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</qemu:commandline>\n"); return 0; } -- 1.8.5.3
On 03/12/2014 10:04 PM, Laine Stump wrote:
This should be squashed into the first patch of the previous "eliminate hardcoded indent" series (which was ACKed but hasn't yet been pushed):
https://www.redhat.com/archives/libvir-list/2014-March/msg00361.html
The initial patch neglected to change the functions that format private metadata in the qemu and lxc domain xml.
I have verified that make check still passes when these changes are squashed into the original. --- src/conf/domain_conf.c | 2 +- src/lxc/lxc_domain.c | 4 ++-- src/qemu/qemu_domain.c | 45 +++++++++++++++++++++++++++------------------ 3 files changed, 30 insertions(+), 21 deletions(-)
ACK. Hmm, your comment about dropping all the virBufferAdjustIndent() calls, free-form output of the text without regards to indentation, then running it through util/virxml.c to parse and then pretty-print the xml, may be more maintainable, even if it eats more CPU cycles. But if we do that, it could be a followup to this series. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/13/2014 04:06 PM, Eric Blake wrote:
On 03/12/2014 10:04 PM, Laine Stump wrote:
This should be squashed into the first patch of the previous "eliminate hardcoded indent" series (which was ACKed but hasn't yet been pushed):
https://www.redhat.com/archives/libvir-list/2014-March/msg00361.html
The initial patch neglected to change the functions that format private metadata in the qemu and lxc domain xml.
I have verified that make check still passes when these changes are squashed into the original. --- src/conf/domain_conf.c | 2 +- src/lxc/lxc_domain.c | 4 ++-- src/qemu/qemu_domain.c | 45 +++++++++++++++++++++++++++------------------ 3 files changed, 30 insertions(+), 21 deletions(-)
ACK.
Hmm, your comment about dropping all the virBufferAdjustIndent() calls, free-form output of the text without regards to indentation, then running it through util/virxml.c to parse and then pretty-print the xml, may be more maintainable, even if it eats more CPU cycles. But if we do that, it could be a followup to this series.
Okay, I made the changes you suggested and pushed this series along with the previous one. It is now illegal to call a virBuffer function with a string that starts with spaces followed by <. I did some experimenting with a pretty-printing virBuffer (as you mentioned above) tonight. After making a virBufferXMLContentAndReset() function that calls virXMLParseString() followed by virXMLNodeToString() (which would magically indent xml that started out unindented), and modifying virDomainDefFormat() to use it, I found the following: 1) if you modify qemuxml2xmltest to run the parse/format operation 300 times for each document, the time to run the test is ~30 seconds when using virBufferContentAndReset() (manual indentation), and 35 seconds when using virBufferXMLContentAndReset() (pretty print). So it adds about 17% to the time for a parse/format roundtrip. 2) virXMLNodeToString() uses double quotes for strings, while we use single quotes everywhere. So all of the tests end up failing (this may have slightly skewed the timing above, since the strcmp would have exited sooner each time). (DV pointed out xmlTextWriter*() functions in libxml as a possible way to overcome the double vs. single quotes problem, but I couldn't immediately figure out how to use it.) 3) I noticed in the code that many of the places where we call virBufferContentAndReset(), we are assuming that it will be successful, just returning the result to the caller, and the caller assumes that if it gets a NULL, an error will have already been reported. But if there is an error in the XML, the pretty-printing version could fail, so every caller will need to be modified to handle an error in this case. 4) libvirt-setuid-rpc-client.la (used by virt-login-shell) needs to have every util .c file it uses added explicitly because we want to give the setuid process access to as little as possible. But it already uses virbuffer.c, and if virbuffer.c calls virXMLParseString(), it will also need to include virxml.c. *and* if virxml.c is needed, then it will also need to link with libxml2. That may be beyond what we want in virt-login-shell. So for the moment I'm going to just let this idea simmer - I think the extra overhead is probably acceptable in return for the easier maintenance, but I'm not sure what to do about the double vs. single quotes, nor whether or not it is acceptable to add so much new code to the footprint of virt-login-shell.
These are never seen externally, only passed into libvirt APIs, so in practice this makes no real difference, but it's good to be consistent. --- tools/virsh-domain.c | 43 ++++++++++++++++++++++++------------------- tools/virsh-pool.c | 40 +++++++++++++++++++++++++--------------- tools/virsh-snapshot.c | 24 +++++++++++++++--------- tools/virsh-volume.c | 26 ++++++++++++++++---------- 4 files changed, 80 insertions(+), 53 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 1d3c5f0..f6a3dc4 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -572,9 +572,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "rawio")) virBufferAddLit(&buf, " rawio='yes'"); virBufferAddLit(&buf, ">\n"); + virBufferAdjustIndent(&buf, 2); if (driver || subdriver || cache) { - virBufferAddLit(&buf, " <driver"); + virBufferAddLit(&buf, "<driver"); if (driver) virBufferAsprintf(&buf, " name='%s'", driver); @@ -587,18 +588,18 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) } if (source) - virBufferAsprintf(&buf, " <source %s='%s'/>\n", + virBufferAsprintf(&buf, "<source %s='%s'/>\n", (isFile) ? "file" : "dev", source); - virBufferAsprintf(&buf, " <target dev='%s'/>\n", target); + virBufferAsprintf(&buf, "<target dev='%s'/>\n", target); if (mode) - virBufferAsprintf(&buf, " <%s/>\n", mode); + virBufferAsprintf(&buf, "<%s/>\n", mode); if (serial) - virBufferAsprintf(&buf, " <serial>%s</serial>\n", serial); + virBufferAsprintf(&buf, "<serial>%s</serial>\n", serial); if (wwn) - virBufferAsprintf(&buf, " <wwn>%s</wwn>\n", wwn); + virBufferAsprintf(&buf, "<wwn>%s</wwn>\n", wwn); if (straddr) { if (str2DiskAddress(straddr, &diskAddr) != 0) { @@ -609,7 +610,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (STRPREFIX((const char *)target, "vd")) { if (diskAddr.type == DISK_ADDR_TYPE_PCI) { virBufferAsprintf(&buf, - " <address type='pci' domain='0x%04x'" + "<address type='pci' domain='0x%04x'" " bus ='0x%02x' slot='0x%02x' function='0x%0x'", diskAddr.addr.pci.domain, diskAddr.addr.pci.bus, diskAddr.addr.pci.slot, diskAddr.addr.pci.function); @@ -623,7 +624,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) } else if (STRPREFIX((const char *)target, "sd")) { if (diskAddr.type == DISK_ADDR_TYPE_SCSI) { virBufferAsprintf(&buf, - " <address type='drive' controller='%d'" + "<address type='drive' controller='%d'" " bus='%d' unit='%d' />\n", diskAddr.addr.scsi.controller, diskAddr.addr.scsi.bus, diskAddr.addr.scsi.unit); @@ -634,7 +635,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) } else if (STRPREFIX((const char *)target, "hd")) { if (diskAddr.type == DISK_ADDR_TYPE_IDE) { virBufferAsprintf(&buf, - " <address type='drive' controller='%d'" + "<address type='drive' controller='%d'" " bus='%d' unit='%d' />\n", diskAddr.addr.ide.controller, diskAddr.addr.ide.bus, diskAddr.addr.ide.unit); @@ -645,6 +646,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) } } + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</disk>\n"); if (virBufferError(&buf)) { @@ -876,25 +878,27 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) /* Make XML of interface */ virBufferAsprintf(&buf, "<interface type='%s'>\n", type); + virBufferAdjustIndent(&buf, 2); if (typ == 1) - virBufferAsprintf(&buf, " <source network='%s'/>\n", source); + virBufferAsprintf(&buf, "<source network='%s'/>\n", source); else if (typ == 2) - virBufferAsprintf(&buf, " <source bridge='%s'/>\n", source); + virBufferAsprintf(&buf, "<source bridge='%s'/>\n", source); if (target != NULL) - virBufferAsprintf(&buf, " <target dev='%s'/>\n", target); + virBufferAsprintf(&buf, "<target dev='%s'/>\n", target); if (mac != NULL) - virBufferAsprintf(&buf, " <mac address='%s'/>\n", mac); + virBufferAsprintf(&buf, "<mac address='%s'/>\n", mac); if (script != NULL) - virBufferAsprintf(&buf, " <script path='%s'/>\n", script); + virBufferAsprintf(&buf, "<script path='%s'/>\n", script); if (model != NULL) - virBufferAsprintf(&buf, " <model type='%s'/>\n", model); + virBufferAsprintf(&buf, "<model type='%s'/>\n", model); if (inboundStr || outboundStr) { - virBufferAddLit(&buf, " <bandwidth>\n"); + virBufferAddLit(&buf, "<bandwidth>\n"); + virBufferAdjustIndent(&buf, 2); if (inboundStr && inbound.average > 0) { - virBufferAsprintf(&buf, " <inbound average='%llu'", inbound.average); + virBufferAsprintf(&buf, "<inbound average='%llu'", inbound.average); if (inbound.peak > 0) virBufferAsprintf(&buf, " peak='%llu'", inbound.peak); if (inbound.burst > 0) @@ -902,14 +906,15 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) virBufferAddLit(&buf, "/>\n"); } if (outboundStr && outbound.average > 0) { - virBufferAsprintf(&buf, " <outbound average='%llu'", outbound.average); + virBufferAsprintf(&buf, "<outbound average='%llu'", outbound.average); if (outbound.peak > 0) virBufferAsprintf(&buf, " peak='%llu'", outbound.peak); if (outbound.burst > 0) virBufferAsprintf(&buf, " burst='%llu'", outbound.burst); virBufferAddLit(&buf, "/>\n"); } - virBufferAddLit(&buf, " </bandwidth>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</bandwidth>\n"); } virBufferAddLit(&buf, "</interface>\n"); diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index b21b682..3d0ea20 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -251,28 +251,34 @@ vshBuildPoolXML(vshControl *ctl, goto cleanup; virBufferAsprintf(&buf, "<pool type='%s'>\n", type); - virBufferAsprintf(&buf, " <name>%s</name>\n", name); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<name>%s</name>\n", name); if (srcHost || srcPath || srcDev || srcFormat || srcName) { - virBufferAddLit(&buf, " <source>\n"); + virBufferAddLit(&buf, "<source>\n"); + virBufferAdjustIndent(&buf, 2); if (srcHost) - virBufferAsprintf(&buf, " <host name='%s'/>\n", srcHost); + virBufferAsprintf(&buf, "<host name='%s'/>\n", srcHost); if (srcPath) - virBufferAsprintf(&buf, " <dir path='%s'/>\n", srcPath); + virBufferAsprintf(&buf, "<dir path='%s'/>\n", srcPath); if (srcDev) - virBufferAsprintf(&buf, " <device path='%s'/>\n", srcDev); + virBufferAsprintf(&buf, "<device path='%s'/>\n", srcDev); if (srcFormat) - virBufferAsprintf(&buf, " <format type='%s'/>\n", srcFormat); + virBufferAsprintf(&buf, "<format type='%s'/>\n", srcFormat); if (srcName) - virBufferAsprintf(&buf, " <name>%s</name>\n", srcName); + virBufferAsprintf(&buf, "<name>%s</name>\n", srcName); - virBufferAddLit(&buf, " </source>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</source>\n"); } if (target) { - virBufferAddLit(&buf, " <target>\n"); - virBufferAsprintf(&buf, " <path>%s</path>\n", target); - virBufferAddLit(&buf, " </target>\n"); + virBufferAddLit(&buf, "<target>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<path>%s</path>\n", target); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</target>\n"); } + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</pool>\n"); if (virBufferError(&buf)) { @@ -1374,15 +1380,19 @@ cmdPoolDiscoverSourcesAs(vshControl * ctl, const vshCmd * cmd ATTRIBUTE_UNUSED) return false; } virBufferAddLit(&buf, "<source>\n"); - virBufferAsprintf(&buf, " <host name='%s'", host); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<host name='%s'", host); if (port) virBufferAsprintf(&buf, " port='%s'", port); virBufferAddLit(&buf, "/>\n"); if (initiator) { - virBufferAddLit(&buf, " <initiator>\n"); - virBufferAsprintf(&buf, " <iqn name='%s'/>\n", initiator); - virBufferAddLit(&buf, " </initiator>\n"); + virBufferAddLit(&buf, "<initiator>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<iqn name='%s'/>\n", initiator); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</initiator>\n"); } + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</source>\n"); if (virBufferError(&buf)) { vshError(ctl, "%s", _("Out of memory")); diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index e37a5b3..6beb25a 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -253,7 +253,7 @@ vshParseSnapshotMemspec(vshControl *ctl, virBufferPtr buf, const char *str) goto cleanup; } - virBufferAddLit(buf, " <memory"); + virBufferAddLit(buf, "<memory"); virBufferEscapeString(buf, " snapshot='%s'", snapshot); virBufferEscapeString(buf, " file='%s'", file); virBufferAddLit(buf, "/>\n"); @@ -293,16 +293,18 @@ vshParseSnapshotDiskspec(vshControl *ctl, virBufferPtr buf, const char *str) goto cleanup; } - virBufferEscapeString(buf, " <disk name='%s'", name); + virBufferEscapeString(buf, "<disk name='%s'", name); if (snapshot) virBufferAsprintf(buf, " snapshot='%s'", snapshot); if (driver || file) { virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); if (driver) - virBufferAsprintf(buf, " <driver type='%s'/>\n", driver); + virBufferAsprintf(buf, "<driver type='%s'/>\n", driver); if (file) - virBufferEscapeString(buf, " <source file='%s'/>\n", file); - virBufferAddLit(buf, " </disk>\n"); + virBufferEscapeString(buf, "<source file='%s'/>\n", file); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</disk>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -424,8 +426,9 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) goto cleanup; virBufferAddLit(&buf, "<domainsnapshot>\n"); - virBufferEscapeString(&buf, " <name>%s</name>\n", name); - virBufferEscapeString(&buf, " <description>%s</description>\n", desc); + virBufferAdjustIndent(&buf, 2); + virBufferEscapeString(&buf, "<name>%s</name>\n", name); + virBufferEscapeString(&buf, "<description>%s</description>\n", desc); if (vshCommandOptStringReq(ctl, cmd, "memspec", &memspec) < 0) goto cleanup; @@ -434,13 +437,16 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) goto cleanup; if (vshCommandOptBool(cmd, "diskspec")) { - virBufferAddLit(&buf, " <disks>\n"); + virBufferAddLit(&buf, "<disks>\n"); + virBufferAdjustIndent(&buf, 2); while ((opt = vshCommandOptArgv(cmd, opt))) { if (vshParseSnapshotDiskspec(ctl, &buf, opt->data) < 0) goto cleanup; } - virBufferAddLit(&buf, " </disks>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</disks>\n"); } + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</domainsnapshot>\n"); if (virBufferError(&buf)) { diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index d7c4823..9bca421 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -212,15 +212,18 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) goto cleanup; virBufferAddLit(&buf, "<volume>\n"); - virBufferAsprintf(&buf, " <name>%s</name>\n", name); - virBufferAsprintf(&buf, " <capacity>%llu</capacity>\n", capacity); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<name>%s</name>\n", name); + virBufferAsprintf(&buf, "<capacity>%llu</capacity>\n", capacity); if (allocationStr) - virBufferAsprintf(&buf, " <allocation>%llu</allocation>\n", allocation); + virBufferAsprintf(&buf, "<allocation>%llu</allocation>\n", allocation); if (format) { - virBufferAddLit(&buf, " <target>\n"); - virBufferAsprintf(&buf, " <format type='%s'/>\n", format); - virBufferAddLit(&buf, " </target>\n"); + virBufferAddLit(&buf, "<target>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<format type='%s'/>\n", format); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</target>\n"); } /* Convert the snapshot parameters into backingStore XML */ @@ -272,18 +275,21 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) } /* Create XML for the backing store */ - virBufferAddLit(&buf, " <backingStore>\n"); - virBufferAsprintf(&buf, " <path>%s</path>\n", snapshotStrVolPath); + virBufferAddLit(&buf, "<backingStore>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<path>%s</path>\n", snapshotStrVolPath); if (snapshotStrFormat) - virBufferAsprintf(&buf, " <format type='%s'/>\n", + virBufferAsprintf(&buf, "<format type='%s'/>\n", snapshotStrFormat); - virBufferAddLit(&buf, " </backingStore>\n"); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</backingStore>\n"); /* Cleanup snapshot allocations */ VIR_FREE(snapshotStrVolPath); virStorageVolFree(snapVol); } + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</volume>\n"); if (virBufferError(&buf)) { -- 1.8.5.3
On 03/12/2014 10:04 PM, Laine Stump wrote:
These are never seen externally, only passed into libvirt APIs, so in practice this makes no real difference, but it's good to be consistent. --- tools/virsh-domain.c | 43 ++++++++++++++++++++++++------------------- tools/virsh-pool.c | 40 +++++++++++++++++++++++++--------------- tools/virsh-snapshot.c | 24 +++++++++++++++--------- tools/virsh-volume.c | 26 ++++++++++++++++---------- 4 files changed, 80 insertions(+), 53 deletions(-)
If nothing else, it keeps the new syntax check happy. ACK.
if (source) - virBufferAsprintf(&buf, " <source %s='%s'/>\n", + virBufferAsprintf(&buf, "<source %s='%s'/>\n", (isFile) ? "file" : "dev",
Wonder if it's worth dropping the extra () while you are here? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
This is also never seen by a human. --- src/qemu/qemu_migration.c | 49 ++++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 54c6fec..0bbec2e 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -535,15 +535,17 @@ qemuMigrationCookieAddNBD(qemuMigrationCookiePtr mig, static void qemuMigrationCookieGraphicsXMLFormat(virBufferPtr buf, qemuMigrationCookieGraphicsPtr grap) { - virBufferAsprintf(buf, " <graphics type='%s' port='%d' listen='%s'", + virBufferAsprintf(buf, "<graphics type='%s' port='%d' listen='%s'", virDomainGraphicsTypeToString(grap->type), grap->port, grap->listen); if (grap->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) virBufferAsprintf(buf, " tlsPort='%d'", grap->tlsPort); if (grap->tlsSubject) { virBufferAddLit(buf, ">\n"); - virBufferEscapeString(buf, " <cert info='subject' value='%s'/>\n", grap->tlsSubject); - virBufferAddLit(buf, " </graphics>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<cert info='subject' value='%s'/>\n", grap->tlsSubject); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</graphics>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -561,23 +563,28 @@ qemuMigrationCookieNetworkXMLFormat(virBufferPtr buf, /* If optr->net[i].vporttype is not set, there is nothing to transfer */ if (optr->net[i].vporttype != VIR_NETDEV_VPORT_PROFILE_NONE) { if (empty) { - virBufferAddLit(buf, " <network>\n"); + virBufferAddLit(buf, "<network>\n"); + virBufferAdjustIndent(buf, 2); empty = false; } - virBufferAsprintf(buf, " <interface index='%zu' vporttype='%s'", + virBufferAsprintf(buf, "<interface index='%zu' vporttype='%s'", i, virNetDevVPortTypeToString(optr->net[i].vporttype)); if (optr->net[i].portdata) { virBufferAddLit(buf, ">\n"); - virBufferEscapeString(buf, " <portdata>%s</portdata>\n", + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<portdata>%s</portdata>\n", optr->net[i].portdata); - virBufferAddLit(buf, " </interface>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</interface>\n"); } else { virBufferAddLit(buf, "/>\n"); } } } - if (!empty) - virBufferAddLit(buf, " </network>\n"); + if (!empty) { + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</network>\n"); + } } @@ -594,14 +601,15 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver, virUUIDFormat(mig->localHostuuid, hostuuidstr); virBufferAddLit(buf, "<qemu-migration>\n"); - virBufferEscapeString(buf, " <name>%s</name>\n", mig->name); - virBufferAsprintf(buf, " <uuid>%s</uuid>\n", uuidstr); - virBufferEscapeString(buf, " <hostname>%s</hostname>\n", mig->localHostname); - virBufferAsprintf(buf, " <hostuuid>%s</hostuuid>\n", hostuuidstr); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<name>%s</name>\n", mig->name); + virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuidstr); + virBufferEscapeString(buf, "<hostname>%s</hostname>\n", mig->localHostname); + virBufferAsprintf(buf, "<hostuuid>%s</hostuuid>\n", hostuuidstr); for (i = 0; i < QEMU_MIGRATION_COOKIE_FLAG_LAST; i++) { if (mig->flagsMandatory & (1 << i)) - virBufferAsprintf(buf, " <feature name='%s'/>\n", + virBufferAsprintf(buf, "<feature name='%s'/>\n", qemuMigrationCookieFlagTypeToString(i)); } @@ -611,16 +619,17 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver, if ((mig->flags & QEMU_MIGRATION_COOKIE_LOCKSTATE) && mig->lockState) { - virBufferAsprintf(buf, " <lockstate driver='%s'>\n", + virBufferAsprintf(buf, "<lockstate driver='%s'>\n", mig->lockDriver); - virBufferAsprintf(buf, " <leases>%s</leases>\n", + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<leases>%s</leases>\n", mig->lockState); - virBufferAddLit(buf, " </lockstate>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</lockstate>\n"); } if ((mig->flags & QEMU_MIGRATION_COOKIE_PERSISTENT) && mig->persistent) { - virBufferAdjustIndent(buf, 2); if (qemuDomainDefFormatBuf(driver, mig->persistent, VIR_DOMAIN_XML_INACTIVE | @@ -628,19 +637,19 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver, VIR_DOMAIN_XML_MIGRATABLE, buf) < 0) return -1; - virBufferAdjustIndent(buf, -2); } if ((mig->flags & QEMU_MIGRATION_COOKIE_NETWORK) && mig->network) qemuMigrationCookieNetworkXMLFormat(buf, mig->network); if ((mig->flags & QEMU_MIGRATION_COOKIE_NBD) && mig->nbd) { - virBufferAddLit(buf, " <nbd"); + virBufferAddLit(buf, "<nbd"); if (mig->nbd->port) virBufferAsprintf(buf, " port='%d'", mig->nbd->port); virBufferAddLit(buf, "/>\n"); } + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</qemu-migration>\n"); return 0; } -- 1.8.5.3
On 03/12/2014 10:04 PM, Laine Stump wrote:
This is also never seen by a human.
except maybe when reading log files
--- src/qemu/qemu_migration.c | 49 ++++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 20 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
This same set of functions is used by the qemu, xen, and lxc drivers' connectSysInfo function. --- src/util/virsysinfo.c | 72 ++++++++++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 32 deletions(-) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index f58122f9..1fa9bef 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -1,7 +1,7 @@ /* * virsysinfo.c: get SMBIOS/sysinfo information from the host * - * Copyright (C) 2010-2013 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * Copyright (C) 2010 Daniel Veillard * * This library is free software; you can redistribute it and/or @@ -880,16 +880,18 @@ virSysinfoBIOSFormat(virBufferPtr buf, virSysinfoDefPtr def) !def->bios_date && !def->bios_release) return; - virBufferAddLit(buf, " <bios>\n"); - virBufferEscapeString(buf, " <entry name='vendor'>%s</entry>\n", + virBufferAddLit(buf, "<bios>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<entry name='vendor'>%s</entry>\n", def->bios_vendor); - virBufferEscapeString(buf, " <entry name='version'>%s</entry>\n", + virBufferEscapeString(buf, "<entry name='version'>%s</entry>\n", def->bios_version); - virBufferEscapeString(buf, " <entry name='date'>%s</entry>\n", + virBufferEscapeString(buf, "<entry name='date'>%s</entry>\n", def->bios_date); - virBufferEscapeString(buf, " <entry name='release'>%s</entry>\n", + virBufferEscapeString(buf, "<entry name='release'>%s</entry>\n", def->bios_release); - virBufferAddLit(buf, " </bios>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</bios>\n"); } static void @@ -900,22 +902,24 @@ virSysinfoSystemFormat(virBufferPtr buf, virSysinfoDefPtr def) !def->system_uuid && !def->system_sku && !def->system_family) return; - virBufferAddLit(buf, " <system>\n"); - virBufferEscapeString(buf, " <entry name='manufacturer'>%s</entry>\n", + virBufferAddLit(buf, "<system>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<entry name='manufacturer'>%s</entry>\n", def->system_manufacturer); - virBufferEscapeString(buf, " <entry name='product'>%s</entry>\n", + virBufferEscapeString(buf, "<entry name='product'>%s</entry>\n", def->system_product); - virBufferEscapeString(buf, " <entry name='version'>%s</entry>\n", + virBufferEscapeString(buf, "<entry name='version'>%s</entry>\n", def->system_version); - virBufferEscapeString(buf, " <entry name='serial'>%s</entry>\n", + virBufferEscapeString(buf, "<entry name='serial'>%s</entry>\n", def->system_serial); - virBufferEscapeString(buf, " <entry name='uuid'>%s</entry>\n", + virBufferEscapeString(buf, "<entry name='uuid'>%s</entry>\n", def->system_uuid); - virBufferEscapeString(buf, " <entry name='sku'>%s</entry>\n", + virBufferEscapeString(buf, "<entry name='sku'>%s</entry>\n", def->system_sku); - virBufferEscapeString(buf, " <entry name='family'>%s</entry>\n", + virBufferEscapeString(buf, "<entry name='family'>%s</entry>\n", def->system_family); - virBufferAddLit(buf, " </system>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</system>\n"); } static void @@ -940,8 +944,8 @@ virSysinfoProcessorFormat(virBufferPtr buf, virSysinfoDefPtr def) !processor->processor_part_number) continue; - virBufferAddLit(buf, " <processor>\n"); - virBufferAdjustIndent(buf, 4); + virBufferAddLit(buf, "<processor>\n"); + virBufferAdjustIndent(buf, 2); virBufferEscapeString(buf, "<entry name='socket_destination'>%s</entry>\n", processor->processor_socket_destination); @@ -965,8 +969,8 @@ virSysinfoProcessorFormat(virBufferPtr buf, virSysinfoDefPtr def) processor->processor_serial_number); virBufferEscapeString(buf, "<entry name='part_number'>%s</entry>\n", processor->processor_part_number); - virBufferAdjustIndent(buf, -4); - virBufferAddLit(buf, " </processor>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</processor>\n"); } } @@ -991,34 +995,36 @@ virSysinfoMemoryFormat(virBufferPtr buf, virSysinfoDefPtr def) !memory->memory_part_number) continue; - virBufferAddLit(buf, " <memory_device>\n"); - virBufferEscapeString(buf, " <entry name='size'>%s</entry>\n", + virBufferAddLit(buf, "<memory_device>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<entry name='size'>%s</entry>\n", memory->memory_size); virBufferEscapeString(buf, - " <entry name='form_factor'>%s</entry>\n", + "<entry name='form_factor'>%s</entry>\n", memory->memory_form_factor); - virBufferEscapeString(buf, " <entry name='locator'>%s</entry>\n", + virBufferEscapeString(buf, "<entry name='locator'>%s</entry>\n", memory->memory_locator); virBufferEscapeString(buf, - " <entry name='bank_locator'>%s</entry>\n", + "<entry name='bank_locator'>%s</entry>\n", memory->memory_bank_locator); - virBufferEscapeString(buf, " <entry name='type'>%s</entry>\n", + virBufferEscapeString(buf, "<entry name='type'>%s</entry>\n", memory->memory_type); virBufferEscapeString(buf, - " <entry name='type_detail'>%s</entry>\n", + "<entry name='type_detail'>%s</entry>\n", memory->memory_type_detail); - virBufferEscapeString(buf, " <entry name='speed'>%s</entry>\n", + virBufferEscapeString(buf, "<entry name='speed'>%s</entry>\n", memory->memory_speed); virBufferEscapeString(buf, - " <entry name='manufacturer'>%s</entry>\n", + "<entry name='manufacturer'>%s</entry>\n", memory->memory_manufacturer); virBufferEscapeString(buf, - " <entry name='serial_number'>%s</entry>\n", + "<entry name='serial_number'>%s</entry>\n", memory->memory_serial_number); virBufferEscapeString(buf, - " <entry name='part_number'>%s</entry>\n", + "<entry name='part_number'>%s</entry>\n", memory->memory_part_number); - virBufferAddLit(buf, " </memory_device>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</memory_device>\n"); } } @@ -1043,12 +1049,14 @@ virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def) } virBufferAsprintf(buf, "<sysinfo type='%s'>\n", type); + virBufferAdjustIndent(buf, 2); virSysinfoBIOSFormat(buf, def); virSysinfoSystemFormat(buf, def); virSysinfoProcessorFormat(buf, def); virSysinfoMemoryFormat(buf, def); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</sysinfo>\n"); if (virBufferError(buf)) { -- 1.8.5.3
On 03/12/2014 10:04 PM, Laine Stump wrote:
This same set of functions is used by the qemu, xen, and lxc drivers' connectSysInfo function. --- src/util/virsysinfo.c | 72 ++++++++++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 32 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
--- src/qemu/qemu_capabilities.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c589a57..47f8bcb 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2539,38 +2539,39 @@ virQEMUCapsSaveCache(virQEMUCapsPtr qemuCaps, const char *filename) size_t i; virBufferAddLit(&buf, "<qemuCaps>\n"); + virBufferAdjustIndent(&buf, 2); - virBufferAsprintf(&buf, " <qemuctime>%llu</qemuctime>\n", + virBufferAsprintf(&buf, "<qemuctime>%llu</qemuctime>\n", (long long)qemuCaps->ctime); - virBufferAsprintf(&buf, " <selfctime>%llu</selfctime>\n", + virBufferAsprintf(&buf, "<selfctime>%llu</selfctime>\n", (long long)virGetSelfLastChanged()); if (qemuCaps->usedQMP) - virBufferAddLit(&buf, " <usedQMP/>\n"); + virBufferAddLit(&buf, "<usedQMP/>\n"); for (i = 0; i < QEMU_CAPS_LAST; i++) { if (virQEMUCapsGet(qemuCaps, i)) { - virBufferAsprintf(&buf, " <flag name='%s'/>\n", + virBufferAsprintf(&buf, "<flag name='%s'/>\n", virQEMUCapsTypeToString(i)); } } - virBufferAsprintf(&buf, " <version>%d</version>\n", + virBufferAsprintf(&buf, "<version>%d</version>\n", qemuCaps->version); - virBufferAsprintf(&buf, " <kvmVersion>%d</kvmVersion>\n", + virBufferAsprintf(&buf, "<kvmVersion>%d</kvmVersion>\n", qemuCaps->kvmVersion); - virBufferAsprintf(&buf, " <arch>%s</arch>\n", + virBufferAsprintf(&buf, "<arch>%s</arch>\n", virArchToString(qemuCaps->arch)); for (i = 0; i < qemuCaps->ncpuDefinitions; i++) { - virBufferEscapeString(&buf, " <cpu name='%s'/>\n", + virBufferEscapeString(&buf, "<cpu name='%s'/>\n", qemuCaps->cpuDefinitions[i]); } for (i = 0; i < qemuCaps->nmachineTypes; i++) { - virBufferEscapeString(&buf, " <machine name='%s'", + virBufferEscapeString(&buf, "<machine name='%s'", qemuCaps->machineTypes[i]); if (qemuCaps->machineAliases[i]) virBufferEscapeString(&buf, " alias='%s'", @@ -2579,6 +2580,7 @@ virQEMUCapsSaveCache(virQEMUCapsPtr qemuCaps, const char *filename) qemuCaps->machineMaxCpus[i]); } + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</qemuCaps>\n"); if (virBufferError(&buf)) -- 1.8.5.3
On 03/12/2014 10:04 PM, Laine Stump wrote:
---
s/elmininate/eliminate/ in the subject
src/qemu/qemu_capabilities.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
This rule wouldn't be able to find any case of a hardcoded indent that was in the middle of a string, but then virBuffer doesn't add indentation in the middle of a string either. --- cfg.mk | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cfg.mk b/cfg.mk index 9ad8cf7..e83fa02 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1,5 +1,5 @@ # Customize Makefile.maint. -*- makefile -*- -# Copyright (C) 2008-2013 Red Hat, Inc. +# Copyright (C) 2008-2014 Red Hat, Inc. # Copyright (C) 2003-2008 Free Software Foundation, Inc. # This program is free software: you can redistribute it and/or modify @@ -517,6 +517,11 @@ sc_prohibit_virBufferAsprintf_with_string_literal: halt='use virBufferAddLit, not virBufferAsprintf, with a string literal' \ $(_sc_search_regexp) +sc_forbid_manual_xml_indent: + @prohibit='virBuffer.*" +<' \ + halt='use virBufferAdjustIndent instead of spaces when indenting xml' \ + $(_sc_search_regexp) + # Not only do they fail to deal well with ipv6, but the gethostby* # functions are also not thread-safe. sc_prohibit_gethostby: -- 1.8.5.3
On 03/12/2014 10:04 PM, Laine Stump wrote:
This rule wouldn't be able to find any case of a hardcoded indent that was in the middle of a string, but then virBuffer doesn't add indentation in the middle of a string either. --- cfg.mk | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
ACK (but I'm biased, as I helped write it)
+sc_forbid_manual_xml_indent: + @prohibit='virBuffer.*" +<' \ + halt='use virBufferAdjustIndent instead of spaces when indenting xml' \
Maybe line up the \ to a common column? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Laine Stump