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@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