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

John Ferlan jferlan at redhat.com
Fri Nov 11 21:41:49 UTC 2016


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.


> 
> Signed-off-by: Eric Farman <farman at linux.vnet.ibm.com>
> ---
>  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...

> +              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 */

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




More information about the libvir-list mailing list