[libvirt] [PATCHv2] conf: fix cannot get mutli value settings when parse controllers XML

[Michal Privoznik]

On 07.01.2015 11:39, Luyao Huang wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1179684
>
> We generate the scsi controller XML like this before
> (actualy this is wrong, we shouldn't set mutli-drivers in different line):
>
>      <controller type='scsi' index='0' model='virtio-scsi'>
>        <driver queues='12'/>
>        <driver cmd_per_lun='123'/>
>        <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
>      </controller>
>
> But this will cause a issue when we parse controllers XML in
> virDomainControllerDefParseXML, we will try to get queues,
> cmd_per_lun,max_sectors settings from XML, we will try to get
> this three values in multi-loop(depend on the number of value
> we set here), then the old value will be covered by new value
> in new loop. The result is we only can get one value settings
> after these loop even we set 3.
>
> The loop is here:
>
>      while (cur != NULL) {
>          if (cur->type == XML_ELEMENT_NODE) {
>              if (xmlStrEqual(cur->name, BAD_CAST "driver")) {
>                  queues = virXMLPropString(cur, "queues");
>                  cmd_per_lun = virXMLPropString(cur, "cmd_per_lun");
>                  max_sectors = virXMLPropString(cur, "max_sectors");
>              }
>          }
>          cur = cur->next;
>      }
>
> this patch will change the XML to this:
>
>      <controller type='scsi' index='0' model='virtio-scsi'>
>        <driver queues='12' cmd_per_lun='123'/>
>        <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
>      </controller>
>
> Signed-off-by: Luyao Huang <lhuang at redhat.com>
> ---
>   src/conf/domain_conf.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b9858cd..b3e9448 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -17120,14 +17120,19 @@ virDomainControllerDefFormat(virBufferPtr buf,
>           virDomainDeviceInfoIsSet(&def->info, flags) || pcihole64) {
>           virBufferAddLit(buf, ">\n");
>           virBufferAdjustIndent(buf, 2);
> -        if (def->queues)
> -            virBufferAsprintf(buf, "<driver queues='%u'/>\n", def->queues);
>
> -        if (def->cmd_per_lun)
> -            virBufferAsprintf(buf, "<driver cmd_per_lun='%u'/>\n", def->cmd_per_lun);
> +        if (def->queues || def->cmd_per_lun || def->max_sectors) {
> +            virBufferAddLit(buf, "<driver");
> +            if (def->queues)
> +                virBufferAsprintf(buf, " queues='%u'", def->queues);
>
> -        if (def->max_sectors)
> -            virBufferAsprintf(buf, "<driver max_sectors='%u'/>\n", def->max_sectors);
> +            if (def->cmd_per_lun)
> +                virBufferAsprintf(buf, " cmd_per_lun='%u'", def->cmd_per_lun);
> +
> +            if (def->max_sectors)
> +                virBufferAsprintf(buf, " max_sectors='%u'", def->max_sectors);
> +            virBufferAddLit(buf, "/>\n");
> +        }
>
>           if (virDomainDeviceInfoIsSet(&def->info, flags) &&
>               virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
>

I've reworded the commit message and pushed.

ACK

Michal