Re: [PATCH v3 03/10] Introduce a "scsi_host" hostdev type

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

 





On 11/11/2016 04:41 PM, John Ferlan wrote:
s/$SUBJ/Introduce framework for hostdev SCSI_Host subsys type

On 11/08/2016 01:26 PM, Eric Farman wrote:
We already have a "scsi" hostdev type, which refers to a single LUN
that is passed through to a guest.  But what of things where multiple
LUNs are passed through via a single SCSI HBA, such as with the
vhost-scsi target?  Create a new hostdev type that will carry this.
s/hostdev type/hostdev subsys type/

The XML will eventually be:

   <hostdev mode='subsystem' type='scsi_host' [managed='no']>
     <source protocol='vhost' wwpn='naa.XXXXXXXXXXXXXXXX'/>
   </hostdev>

NB: The "hostdev" RNG definition does list an optional "<address>"
element which it doesn't seem you save in the guest config during any of
these patches. I do see a remnant of CCW for the running guest, but
nothing for PCI (which in the end is what's used - vhost-scsi-pci or
vhost-scsi-ccw). In any case, having "predictable" or "saved" addresses
is something we've found through history (USB and more recently Memory
dimms) to be a good idea.

The point being - I think you need to make sure that if not supplied,
then an address is generated (whether it's for PCI or CCW). While not in
this patch per se, it is something you'll need to handle in patch 7.

Okay.



Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxxxxxxx>
---
  src/conf/domain_conf.c              | 12 +++++++++++-
  src/conf/domain_conf.h              | 18 ++++++++++++++++++
  src/qemu/qemu_cgroup.c              |  7 +++++++
  src/qemu/qemu_hotplug.c             |  2 ++
  src/security/security_apparmor.c    |  4 ++++
  src/security/security_dac.c         |  8 ++++++++
  src/security/security_selinux.c     |  8 ++++++++
  tests/domaincapsschemadata/full.xml |  1 +
  8 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 043f0e2..b8a3366 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -647,7 +647,8 @@ VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST,
  VIR_ENUM_IMPL(virDomainHostdevSubsys, VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST,
                "usb",
                "pci",
-              "scsi")
+              "scsi",
+              "scsi_host")
VIR_ENUM_IMPL(virDomainHostdevSubsysPCIBackend,
                VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST,
@@ -661,6 +662,11 @@ VIR_ENUM_IMPL(virDomainHostdevSubsysSCSIProtocol,
                "adapter",
                "iscsi")
+VIR_ENUM_IMPL(virDomainHostdevSubsysHostProtocol,
In order to "follow" convention for structure names already

s/SubsysHostProtocol/SubsysSCSIHostProtocol/

Of course this has ramifications beyond this patch...

I avoided this (and many of the subsequent comments below) because the existing struct virDomainHostdevSubsysSCSI has a union for both iSCSI and it's version of host, and the latter is of course named virDomainHostdevSubsysSCSIHost(Ptr). If that's not much of a concern, then I'll go shake out the variations of host->scsihost you describe below.

 - Eric


+              VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_LAST,
+              "none",
+              "vhost")
[1] because of this...

+
  VIR_ENUM_IMPL(virDomainHostdevCaps, VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST,
                "storage",
                "misc",
@@ -13016,6 +13022,8 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt,
              break;
          case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
              break;
+        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
+            break;
          case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
              goto error;
              break;
@@ -13899,6 +13907,8 @@ virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a,
              return virDomainHostdevMatchSubsysSCSIiSCSI(a, b);
          else
              return virDomainHostdevMatchSubsysSCSIHost(a, b);
+    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
+        /* Fall through for now */
      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
          return 0;
      }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 541b600..8b03561 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -294,6 +294,7 @@ typedef enum {
      VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB,
      VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI,
      VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI,
+    VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST,
[1] - I think this should use SCSI_HOST as well.

s/HOST/SCSI_HOST

There's lots of impact from hereonin...

VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST
  } virDomainHostdevSubsysType;
@@ -368,6 +369,22 @@ struct _virDomainHostdevSubsysSCSI {
      } u;
  };
+typedef enum {
+    VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_NONE,
+    VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_VHOST,
+
+    VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_LAST,
These would all need s/HOST/SCSI_HOST/

+} virDomainHostdevSubsysHostProtocolType;
+
+VIR_ENUM_DECL(virDomainHostdevSubsysHostProtocol)
And these would have s/Host/SCSIHost/
+
+typedef struct _virDomainHostdevSubsysHost virDomainHostdevSubsysHost;
+typedef virDomainHostdevSubsysHost *virDomainHostdevSubsysHostPtr;
+struct _virDomainHostdevSubsysHost {
s/Host/SCSIHost/

+    int protocol;
We've been trying to add the enum in a comment e.g.

     int protocol; /* enum virDomainHostdevSubsysSCSIHostProtocolType */

Hrm, I thought I'd included that.  Maybe I dreamt it.  :)


+    char *wwpn;
+};
+
  typedef struct _virDomainHostdevSubsys virDomainHostdevSubsys;
  typedef virDomainHostdevSubsys *virDomainHostdevSubsysPtr;
  struct _virDomainHostdevSubsys {
@@ -376,6 +393,7 @@ struct _virDomainHostdevSubsys {
          virDomainHostdevSubsysUSB usb;
          virDomainHostdevSubsysPCI pci;
          virDomainHostdevSubsysSCSI scsi;
+        virDomainHostdevSubsysHost host;
s/SubsysHost/SubsysSCSIHost/

and

s/ host;/ scsi_host;/


I think the compiler will find the rest ;-)

John

      } u;
  };
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 1443f7e..ee31d14 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -376,6 +376,10 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm,
              break;
          }
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: {
+            break;
+        }
+
          case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
              break;
          }
@@ -440,6 +444,9 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm,
          case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
              /* nothing to tear down for SCSI */
              break;
+        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
+            /* nothing to tear down for scsi_host */
+            break;
          case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
              break;
          }
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e06862c..2d6b086 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3626,6 +3626,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
          qemuDomainRemoveSCSIHostDevice(driver, vm, hostdev);
          break;
+    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
+        break;
      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
          break;
      }
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index beddf6d..e7e3c8c 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -909,6 +909,10 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
          break;
      }
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: {
+        /* Fall through for now */
+    }
+
      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
          ret = 0;
          break;
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index fd74e8b..eba2a87 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -676,6 +676,10 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
          break;
      }
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: {
+        /* Fall through for now */
+    }
+
      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
          ret = 0;
          break;
@@ -805,6 +809,10 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
          break;
      }
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: {
+        /* Fall through for now */
+    }
+
      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
          ret = 0;
          break;
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 89c93dc..a94bba3 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1498,6 +1498,10 @@ virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManagerPtr mgr,
          break;
      }
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: {
+        /* Fall through for now */
+    }
+
      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
          ret = 0;
          break;
@@ -1700,6 +1704,10 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManagerPtr mgr,
          break;
      }
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: {
+        /* Fall through for now */
+    }
+
      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
          ret = 0;
          break;
diff --git a/tests/domaincapsschemadata/full.xml b/tests/domaincapsschemadata/full.xml
index eaf6eb6..6abd499 100644
--- a/tests/domaincapsschemadata/full.xml
+++ b/tests/domaincapsschemadata/full.xml
@@ -87,6 +87,7 @@
          <value>usb</value>
          <value>pci</value>
          <value>scsi</value>
+        <value>scsi_host</value>
        </enum>
        <enum name='capsType'>
          <value>storage</value>


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