This looks good to me.
Acked-by: Kyle Mestery <kmestery@xxxxxxxxx>
On Aug 14, 2012, at 2:04 AM, Laine Stump wrote:
> This function has several calls to increase the buffer indent by 6,
> then decrease it again, then increase, then decrease. Additionally,
> there were several printfs that had 6 spaces at the beginning of the
> line.
>
> virDomainActualNetDefFormat, which is called by virDomainNetDefFormat,
> had similar ugliness.
>
> This patch changes both functions to just increase the indent at the
> beginning, decrease it at (well, just before*) the end, and remove all
> of the occurences of 6/8 spaces at the beginning of lines.
>
> *The indent had to be reset before the end of the function because
> virDomainDeviceInfoFormat assumes a 0 indent and is called from many
> other places, and I didn't want to do an overhaul of every caller of
> that function. A separate patch to switch all of domain_conf.c would
> be a useful exercise, but my current goal is unrelated to that, so
> I'll leave it for another day.
> ---
> src/conf/domain_conf.c | 73 +++++++++++++++++++++++---------------------------
> 1 file changed, 33 insertions(+), 40 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 5990634..8f1f244 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -11560,21 +11560,22 @@ virDomainActualNetDefFormat(virBufferPtr buf,
> return -1;
> }
>
> - virBufferAsprintf(buf, " <actual type='%s'", type);
> + virBufferAsprintf(buf, "<actual type='%s'", type);
> if (def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV &&
> def->data.hostdev.def.managed) {
> virBufferAddLit(buf, " managed='yes'");
> }
> virBufferAddLit(buf, ">\n");
>
> + virBufferAdjustIndent(buf, 2);
> switch (def->type) {
> case VIR_DOMAIN_NET_TYPE_BRIDGE:
> - virBufferEscapeString(buf, " <source bridge='%s'/>\n",
> + virBufferEscapeString(buf, "<source bridge='%s'/>\n",
> def->data.bridge.brname);
> break;
>
> case VIR_DOMAIN_NET_TYPE_DIRECT:
> - virBufferAddLit(buf, " <source");
> + virBufferAddLit(buf, "<source");
> if (def->data.direct.linkdev)
> virBufferEscapeString(buf, " dev='%s'",
> def->data.direct.linkdev);
> @@ -11590,12 +11591,10 @@ virDomainActualNetDefFormat(virBufferPtr buf,
> break;
>
> case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> - virBufferAdjustIndent(buf, 8);
> if (virDomainHostdevSourceFormat(buf, &def->data.hostdev.def,
> flags, true) < 0) {
> return -1;
> }
> - virBufferAdjustIndent(buf, -8);
> break;
>
> case VIR_DOMAIN_NET_TYPE_NETWORK:
> @@ -11606,14 +11605,13 @@ virDomainActualNetDefFormat(virBufferPtr buf,
> return -1;
> }
>
> - virBufferAdjustIndent(buf, 8);
> if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0)
> return -1;
> if (virNetDevBandwidthFormat(def->bandwidth, buf) < 0)
> return -1;
> - virBufferAdjustIndent(buf, -8);
>
> - virBufferAddLit(buf, " </actual>\n");
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</actual>\n");
> return 0;
> }
>
> @@ -11637,14 +11635,15 @@ virDomainNetDefFormat(virBufferPtr buf,
> }
> virBufferAddLit(buf, ">\n");
>
> + virBufferAdjustIndent(buf, 6);
> virBufferAsprintf(buf,
> - " <mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n",
> + "<mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n",
> def->mac.addr[0], def->mac.addr[1], def->mac.addr[2],
> def->mac.addr[3], def->mac.addr[4], def->mac.addr[5]);
>
> switch (def->type) {
> case VIR_DOMAIN_NET_TYPE_NETWORK:
> - virBufferEscapeString(buf, " <source network='%s'",
> + virBufferEscapeString(buf, "<source network='%s'",
> def->data.network.name);
> virBufferEscapeString(buf, " portgroup='%s'",
> def->data.network.portgroup);
> @@ -11655,39 +11654,41 @@ virDomainNetDefFormat(virBufferPtr buf,
> break;
>
> case VIR_DOMAIN_NET_TYPE_ETHERNET:
> - virBufferEscapeString(buf, " <source dev='%s'/>\n",
> + virBufferEscapeString(buf, "<source dev='%s'/>\n",
> def->data.ethernet.dev);
> if (def->data.ethernet.ipaddr)
> - virBufferAsprintf(buf, " <ip address='%s'/>\n",
> + virBufferAsprintf(buf, "<ip address='%s'/>\n",
> def->data.ethernet.ipaddr);
> break;
>
> case VIR_DOMAIN_NET_TYPE_BRIDGE:
> - virBufferEscapeString(buf, " <source bridge='%s'/>\n",
> + virBufferEscapeString(buf, "<source bridge='%s'/>\n",
> def->data.bridge.brname);
> - if (def->data.bridge.ipaddr)
> - virBufferAsprintf(buf, " <ip address='%s'/>\n",
> + if (def->data.bridge.ipaddr) {
> + virBufferAsprintf(buf, "<ip address='%s'/>\n",
> def->data.bridge.ipaddr);
> + }
> break;
>
> case VIR_DOMAIN_NET_TYPE_SERVER:
> case VIR_DOMAIN_NET_TYPE_CLIENT:
> case VIR_DOMAIN_NET_TYPE_MCAST:
> - if (def->data.socket.address)
> - virBufferAsprintf(buf, " <source address='%s' port='%d'/>\n",
> + if (def->data.socket.address) {
> + virBufferAsprintf(buf, "<source address='%s' port='%d'/>\n",
> def->data.socket.address, def->data.socket.port);
> - else
> - virBufferAsprintf(buf, " <source port='%d'/>\n",
> + } else {
> + virBufferAsprintf(buf, "<source port='%d'/>\n",
> def->data.socket.port);
> + }
> break;
>
> case VIR_DOMAIN_NET_TYPE_INTERNAL:
> - virBufferEscapeString(buf, " <source name='%s'/>\n",
> + virBufferEscapeString(buf, "<source name='%s'/>\n",
> def->data.internal.name);
> break;
>
> case VIR_DOMAIN_NET_TYPE_DIRECT:
> - virBufferEscapeString(buf, " <source dev='%s'",
> + virBufferEscapeString(buf, "<source dev='%s'",
> def->data.direct.linkdev);
> virBufferAsprintf(buf, " mode='%s'",
> virNetDevMacVLanModeTypeToString(def->data.direct.mode));
> @@ -11695,12 +11696,10 @@ virDomainNetDefFormat(virBufferPtr buf,
> break;
>
> case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> - virBufferAdjustIndent(buf, 6);
> if (virDomainHostdevSourceFormat(buf, &def->data.hostdev.def,
> flags, true) < 0) {
> return -1;
> }
> - virBufferAdjustIndent(buf, -6);
> break;
>
> case VIR_DOMAIN_NET_TYPE_USER:
> @@ -11708,26 +11707,22 @@ virDomainNetDefFormat(virBufferPtr buf,
> break;
> }
>
> - virBufferAdjustIndent(buf, 6);
> if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0)
> return -1;
> - virBufferAdjustIndent(buf, -6);
> -
> - virBufferEscapeString(buf, " <script path='%s'/>\n",
> + virBufferEscapeString(buf, "<script path='%s'/>\n",
> def->script);
> if (def->ifname &&
> !((flags & VIR_DOMAIN_XML_INACTIVE) &&
> (STRPREFIX(def->ifname, VIR_NET_GENERATED_PREFIX)))) {
> /* Skip auto-generated target names for inactive config. */
> - virBufferEscapeString(buf, " <target dev='%s'/>\n",
> - def->ifname);
> + virBufferEscapeString(buf, "<target dev='%s'/>\n", def->ifname);
> }
> if (def->model) {
> - virBufferEscapeString(buf, " <model type='%s'/>\n",
> + virBufferEscapeString(buf, "<model type='%s'/>\n",
> def->model);
> if (STREQ(def->model, "virtio") &&
> (def->driver.virtio.name || def->driver.virtio.txmode)) {
> - virBufferAddLit(buf, " <driver");
> + virBufferAddLit(buf, "<driver");
> if (def->driver.virtio.name) {
> virBufferAsprintf(buf, " name='%s'",
> virDomainNetBackendTypeToString(def->driver.virtio.name));
> @@ -11748,27 +11743,25 @@ virDomainNetDefFormat(virBufferPtr buf,
> }
> }
> if (def->filter) {
> - virBufferAdjustIndent(buf, 6);
> if (virNWFilterFormatParamAttributes(buf, def->filterparams,
> def->filter) < 0)
> return -1;
> - virBufferAdjustIndent(buf, -6);
> }
>
> if (def->tune.sndbuf_specified) {
> - virBufferAddLit(buf, " <tune>\n");
> - virBufferAsprintf(buf, " <sndbuf>%lu</sndbuf>\n",
> - def->tune.sndbuf);
> - virBufferAddLit(buf, " </tune>\n");
> + virBufferAddLit(buf, "<tune>\n");
> + virBufferAsprintf(buf, " <sndbuf>%lu</sndbuf>\n", def->tune.sndbuf);
> + virBufferAddLit(buf, "</tune>\n");
> }
>
> - if (def->linkstate)
> - virBufferAsprintf(buf, " <link state='%s'/>\n",
> + if (def->linkstate) {
> + virBufferAsprintf(buf, "<link state='%s'/>\n",
> virDomainNetInterfaceLinkStateTypeToString(def->linkstate));
> + }
>
> - virBufferAdjustIndent(buf, 6);
> if (virNetDevBandwidthFormat(def->bandwidth, buf) < 0)
> return -1;
> +
> virBufferAdjustIndent(buf, -6);
>
> if (virDomainDeviceInfoFormat(buf, &def->info,
> --
> 1.7.11.2
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list