Re: [PATCH 3/4] conf: Use existing SCSI hostdev model to create new

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12/06/2017 08:08 AM, 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.

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;
+    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)
+                newModel = model;

What's the harm if the last controller were undefined? Wouldn't it get populated down the road anyway if one were not set at this point (we send -1 today, after all). That could eliminate one of the two virDomainControllerModelSCSI variables here, I would think.

But, either way I think it's fine.

Reviewed-by: Eric Farman <farman@xxxxxxxxxxxxxxxxxx>

          }
      }

@@ -17702,7 +17712,8 @@ virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def)
          return 0;

      for (i = 0; i <= maxController; i++) {
-        if (virDomainDefMaybeAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_SCSI, i, -1) < 0)
+        if (virDomainDefMaybeAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_SCSI,
+                                           i, newModel) < 0)
              return -1;
      }

diff --git a/tests/qemuxml2xmloutdata/hostdev-scsi-autogen-address.xml b/tests/qemuxml2xmloutdata/hostdev-scsi-autogen-address.xml
index 8e93056ee..cea212b64 100644
--- a/tests/qemuxml2xmloutdata/hostdev-scsi-autogen-address.xml
+++ b/tests/qemuxml2xmloutdata/hostdev-scsi-autogen-address.xml
@@ -29,7 +29,7 @@
        <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
      </controller>
      <controller type='pci' index='0' model='pci-root'/>
-    <controller type='scsi' index='1'>
+    <controller type='scsi' index='1' model='virtio-scsi'>
        <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
      </controller>
      <input type='mouse' bus='ps2'/>


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux