On Wed, Dec 20, 2017 at 01:52:16PM +0100, Ján Tomko wrote:
On Wed, Dec 06, 2017 at 08:08:05AM -0500, John Ferlan wrote:In virDomainDefMaybeAddHostdevSCSIcontroller when we add a new controller because someone neglected to add one or we're adding one because the existing one is full, we should copy over the model number from the existing controller since whatever we create should at least have the same characteristics as the one we cannot use because it's full. NB: This affects the existing hostdev-scsi-autogen-address test which would add a default ('lsi') SCSI controller for the various scsi_host's that would create a controller for the hostdev.Yet the change adds 'virtio-scsi'.Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> --- src/conf/domain_conf.c | 13 ++++++++++++- tests/qemuxml2xmloutdata/hostdev-scsi-autogen-address.xml | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 66e21c4bd..61b4a0075 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17689,12 +17689,22 @@ virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def) size_t i; int maxController = -1; virDomainHostdevDefPtr hostdev; + virDomainControllerModelSCSI model = -1;This declaration can be moved inside the if to reduce its scope. (I'm with Andrea on this [0])
Also, assigning -1 to an unsigned variable won't stick.
+ virDomainControllerModelSCSI newModel = -1; for (i = 0; i < def->nhostdevs; i++) { hostdev = def->hostdevs[i]; if (virHostdevIsSCSIDevice(hostdev) && (int)hostdev->info->addr.drive.controller > maxController) { maxController = hostdev->info->addr.drive.controller; + /* We may be creating a new controller because this one is full. + * So let's grab the model from it and update the model we're + * going to add as long as this one isn't undefined. The premise + * being keeping the same controller model for all SCSI hostdevs. */ + model = virDomainDeviceFindControllerModel(def, hostdev->info, + VIR_DOMAIN_CONTROLLER_TYPE_SCSI); + if (model != -1)
This condition is always true according to my compiler. Jan
+ newModel = model; } }ACK Jan [0] https://www.redhat.com/archives/libvir-list/2017-December/msg00563.html
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
