[libvirt] [PATCH v3 03/10] Introduce a "scsi_host" hostdev type
Eric Farman
farman at linux.vnet.ibm.com
Mon Nov 14 13:31:20 UTC 2016
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 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...
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>
>>
More information about the libvir-list
mailing list