Re: [libvirt] [PATCH 18/25] qemu: Refactor the helpers to track shared scsi host device

2013-05-16 Thread Osier Yang 

On 08/05/13 02:57, John Ferlan wrote:

On 05/03/2013 02:07 PM, Osier Yang wrote:

This changes the helpers qemu{Add,Remove}SharedDisk into
qemu{Add,Remove}SharedDevice, as most of the code in the helpers
can be reused for scsi host device.

To track the shared scsi host device, first it finds out the
device path (e.g. /dev/s[dr]*) which is mapped to the sg device,
and use device ID of the found device path (/dev/s[dr]*) as the
hash key. This is because of the device ID is not unique between
between /dev/s[dr]* and /dev/sg*, e.g.

% sg_map
/dev/sg0  /dev/sda
/dev/sg1  /dev/sr0

% ls -l /dev/sda
brw-rw. 1 root disk 8, 0 May  2 19:26 /dev/sda

%ls -l /dev/sg0
crw-rw. 1 root disk 21, 0 May  2 19:26 /dev/sg0
---
  src/qemu/qemu_conf.c| 143 
  src/qemu/qemu_conf.h|  20 +++
  src/qemu/qemu_driver.c  |  16 +++---
  src/qemu/qemu_process.c |  19 +--
  4 files changed, 140 insertions(+), 58 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 244795d..ebcd176 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1084,47 +1084,80 @@ cleanup:
  return NULL;
  }
  
-/* qemuAddSharedDisk:

+/* qemuAddSharedDevice:
   * @driver: Pointer to qemu driver struct
- * @disk: The disk def
+ * @dev: The device def
   * @name: The domain name
   *
   * Increase ref count and add the domain name into the list which
- * records all the domains that use the shared disk if the entry
+ * records all the domains that use the shared device if the entry
   * already exists, otherwise add a new entry.
   */
  int
-qemuAddSharedDisk(virQEMUDriverPtr driver,
-  virDomainDiskDefPtr disk,
-  const char *name)
+qemuAddSharedDevice(virQEMUDriverPtr driver,
+virDomainDeviceDefPtr dev,
+const char *name)
  {
  qemuSharedDeviceEntry *entry = NULL;
  qemuSharedDeviceEntry *new_entry = NULL;
+virDomainDiskDefPtr disk = NULL;
+virDomainHostdevDefPtr hostdev = NULL;
+char *dev_name = NULL;
+char *dev_path = NULL;
  char *key = NULL;
  int ret = -1;
  
-/* Currently the only conflicts we have to care about

- * for the shared disk is sgio setting, which is only
- * valid for block disk.
+/* Currently the only conflicts we have to care about for
+ * the shared disk and shared host device is sgio setting,
+ * which is only valid for block disk and scsi host device.
   */
-if (!disk-shared ||
-!disk-src ||
-(disk-type != VIR_DOMAIN_DISK_TYPE_BLOCK 
- !(disk-type == VIR_DOMAIN_DISK_TYPE_VOLUME 
-   disk-srcpool 
-   disk-srcpool-voltype == VIR_STORAGE_VOL_BLOCK)))
+if (dev-type == VIR_DOMAIN_DEVICE_DISK) {
+disk = dev-data.disk;
+
+if (disk-shared ||
+!disk-src ||
+(disk-type != VIR_DOMAIN_DISK_TYPE_BLOCK 
+ !(disk-type == VIR_DOMAIN_DISK_TYPE_VOLUME 
+   disk-srcpool 
+   disk-srcpool-voltype == VIR_STORAGE_VOL_BLOCK)))
+return 0;
+} else if (dev-type == VIR_DOMAIN_DEVICE_HOSTDEV) {
+hostdev = dev-data.hostdev;
+
+if (!hostdev-shareable ||
+(hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS 
+ hostdev-source.subsys.type == 
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI))

And this is the controller that's being OR'd so I think this is right,
but making sure :-)

Over time HPVM was asked to allow a whole controller to be shareable
because the guest would control/manage the luns...



+return 0;
+} else {
  return 0;
+}
  
  qemuDriverLock(driver);

-if (qemuCheckSharedDisk(driver-sharedDevices, disk)  0)
-goto cleanup;
+if (dev-type == VIR_DOMAIN_DEVICE_DISK) {
+if (qemuCheckSharedDisk(driver-sharedDevices, disk)  0)
+goto cleanup;
  
-if (!(key = qemuGetSharedDeviceKey(disk-src)))

-goto cleanup;
+if (!(key = qemuGetSharedDeviceKey(disk-src)))
+goto cleanup;
+} else {
+if (!(dev_name = 
virSCSIDeviceGetDevName(hostdev-source.subsys.u.scsi.adapter,
+ 
hostdev-source.subsys.u.scsi.bus,
+ 
hostdev-source.subsys.u.scsi.target,
+ 
hostdev-source.subsys.u.scsi.unit)))
+goto cleanup;
+
+if (virAsprintf(dev_path, /dev/%s, dev_name)  0) {
+virReportOOMError();
+goto cleanup;
+}
+
+if (!(key = qemuGetSharedDeviceKey(dev_path)))
+goto cleanup;
+}
  
  if ((entry = virHashLookup(driver-sharedDevices, key))) {

-/* Nothing to do if the shared disk is already recorded
- * in the table.
+/* Nothing to do if the shared scsi host device is already
+ * recorded in the table.
   */
  if

Re: [libvirt] [PATCH 18/25] qemu: Refactor the helpers to track shared scsi host device

2013-05-07 Thread John Ferlan 
On 05/03/2013 02:07 PM, Osier Yang wrote:
 This changes the helpers qemu{Add,Remove}SharedDisk into
 qemu{Add,Remove}SharedDevice, as most of the code in the helpers
 can be reused for scsi host device.
 
 To track the shared scsi host device, first it finds out the
 device path (e.g. /dev/s[dr]*) which is mapped to the sg device,
 and use device ID of the found device path (/dev/s[dr]*) as the
 hash key. This is because of the device ID is not unique between
 between /dev/s[dr]* and /dev/sg*, e.g.
 
 % sg_map
 /dev/sg0  /dev/sda
 /dev/sg1  /dev/sr0
 
 % ls -l /dev/sda
 brw-rw. 1 root disk 8, 0 May  2 19:26 /dev/sda
 
 %ls -l /dev/sg0
 crw-rw. 1 root disk 21, 0 May  2 19:26 /dev/sg0
 ---
  src/qemu/qemu_conf.c| 143 
 
  src/qemu/qemu_conf.h|  20 +++
  src/qemu/qemu_driver.c  |  16 +++---
  src/qemu/qemu_process.c |  19 +--
  4 files changed, 140 insertions(+), 58 deletions(-)
 
 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
 index 244795d..ebcd176 100644
 --- a/src/qemu/qemu_conf.c
 +++ b/src/qemu/qemu_conf.c
 @@ -1084,47 +1084,80 @@ cleanup:
  return NULL;
  }
  
 -/* qemuAddSharedDisk:
 +/* qemuAddSharedDevice:
   * @driver: Pointer to qemu driver struct
 - * @disk: The disk def
 + * @dev: The device def
   * @name: The domain name
   *
   * Increase ref count and add the domain name into the list which
 - * records all the domains that use the shared disk if the entry
 + * records all the domains that use the shared device if the entry
   * already exists, otherwise add a new entry.
   */
  int
 -qemuAddSharedDisk(virQEMUDriverPtr driver,
 -  virDomainDiskDefPtr disk,
 -  const char *name)
 +qemuAddSharedDevice(virQEMUDriverPtr driver,
 +virDomainDeviceDefPtr dev,
 +const char *name)
  {
  qemuSharedDeviceEntry *entry = NULL;
  qemuSharedDeviceEntry *new_entry = NULL;
 +virDomainDiskDefPtr disk = NULL;
 +virDomainHostdevDefPtr hostdev = NULL;
 +char *dev_name = NULL;
 +char *dev_path = NULL;
  char *key = NULL;
  int ret = -1;
  
 -/* Currently the only conflicts we have to care about
 - * for the shared disk is sgio setting, which is only
 - * valid for block disk.
 +/* Currently the only conflicts we have to care about for
 + * the shared disk and shared host device is sgio setting,
 + * which is only valid for block disk and scsi host device.
   */
 -if (!disk-shared ||
 -!disk-src ||
 -(disk-type != VIR_DOMAIN_DISK_TYPE_BLOCK 
 - !(disk-type == VIR_DOMAIN_DISK_TYPE_VOLUME 
 -   disk-srcpool 
 -   disk-srcpool-voltype == VIR_STORAGE_VOL_BLOCK)))
 +if (dev-type == VIR_DOMAIN_DEVICE_DISK) {
 +disk = dev-data.disk;
 +
 +if (disk-shared ||
 +!disk-src ||
 +(disk-type != VIR_DOMAIN_DISK_TYPE_BLOCK 
 + !(disk-type == VIR_DOMAIN_DISK_TYPE_VOLUME 
 +   disk-srcpool 
 +   disk-srcpool-voltype == VIR_STORAGE_VOL_BLOCK)))
 +return 0;
 +} else if (dev-type == VIR_DOMAIN_DEVICE_HOSTDEV) {
 +hostdev = dev-data.hostdev;
 +
 +if (!hostdev-shareable ||
 +(hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS 
 + hostdev-source.subsys.type == 
 VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI))

And this is the controller that's being OR'd so I think this is right,
but making sure :-)

Over time HPVM was asked to allow a whole controller to be shareable
because the guest would control/manage the luns...


 +return 0;
 +} else {
  return 0;
 +}
  
  qemuDriverLock(driver);
 -if (qemuCheckSharedDisk(driver-sharedDevices, disk)  0)
 -goto cleanup;
 +if (dev-type == VIR_DOMAIN_DEVICE_DISK) {
 +if (qemuCheckSharedDisk(driver-sharedDevices, disk)  0)
 +goto cleanup;
  
 -if (!(key = qemuGetSharedDeviceKey(disk-src)))
 -goto cleanup;
 +if (!(key = qemuGetSharedDeviceKey(disk-src)))
 +goto cleanup;
 +} else {
 +if (!(dev_name = 
 virSCSIDeviceGetDevName(hostdev-source.subsys.u.scsi.adapter,
 + 
 hostdev-source.subsys.u.scsi.bus,
 + 
 hostdev-source.subsys.u.scsi.target,
 + 
 hostdev-source.subsys.u.scsi.unit)))
 +goto cleanup;
 +
 +if (virAsprintf(dev_path, /dev/%s, dev_name)  0) {
 +virReportOOMError();
 +goto cleanup;
 +}
 +
 +if (!(key = qemuGetSharedDeviceKey(dev_path)))
 +goto cleanup;
 +}
  
  if ((entry = virHashLookup(driver-sharedDevices, key))) {
 -/* Nothing to do if the shared disk is already recorded
 - * in the table.
 +/* Nothing to do if the shared scsi host device is already
 +