Re: [PATCH 1/2] hyperv: XML parsing of storage volumes

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

 



On 11/19/20 8:38 AM, Daniel P. Berrangé wrote:
On Wed, Nov 18, 2020 at 01:48:24PM -0500, Matt Coleman wrote:
dumpxml can now serialize:
* floppy drives
* file-backed and device-backed disk drives
* images mounted to virtual CD/DVD drives
* IDE and SCSI controllers

Co-authored-by: Sri Ramanujam <sramanujam@xxxxxxxxx>
Signed-off-by: Matt Coleman <matt@xxxxxxxxx>
---
  src/hyperv/hyperv_driver.c            | 408 +++++++++++++++++++++++++-
  src/hyperv/hyperv_driver.h            |   3 +
  src/hyperv/hyperv_wmi.c               |  45 +++
  src/hyperv/hyperv_wmi.h               |   8 +
  src/hyperv/hyperv_wmi_classes.h       |  19 ++
  src/hyperv/hyperv_wmi_generator.input | 134 +++++++++
  6 files changed, 616 insertions(+), 1 deletion(-)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 40739595ac..ce9ba2a02a 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -293,6 +293,385 @@ hypervCapsInit(hypervPrivate *priv)
      return NULL;
  }
+/*
+ * Virtual device functions
+ */
+static int
+hypervGetDeviceParentRasdFromDeviceId(const char *parentDeviceId,
+                                      Msvm_ResourceAllocationSettingData *list,
+                                      Msvm_ResourceAllocationSettingData **out)
+{
+    Msvm_ResourceAllocationSettingData *entry = list;
+    g_autofree char *escapedDeviceId = NULL;
+    *out = NULL;
+
+    while (entry) {
+        escapedDeviceId = g_strdup_printf("%s\"", entry->data->InstanceID);
+        escapedDeviceId = virStringReplace(escapedDeviceId, "\\", "\\\\");
This leaks the original escapedDeviceId pointer.  The autofree only
runs when variables go out of scope, not when you overwrite pointers.

It'll also leak on every loop iteration.

+
+        if (g_str_has_suffix(parentDeviceId, escapedDeviceId)) {
+            *out = entry;
+            break;
+        }
+
+        entry = entry->next;
+    }
+
+    if (*out)
+        return 0;
+
+    return -1;
+}

diff --git a/src/hyperv/hyperv_driver.h b/src/hyperv/hyperv_driver.h
index 8099b5714b..e49550a2c8 100644
--- a/src/hyperv/hyperv_driver.h
+++ b/src/hyperv/hyperv_driver.h
@@ -22,4 +22,7 @@
#pragma once +#define HYPERV_MAX_SCSI_CONTROLLERS 4
+#define HYPERV_MAX_IDE_CONTROLLERS 2
Is it really possible to have 2 IDE controllers ?


I know nothing about hyperv, but qemu does allow multiple IDE controllers, and I even once wrote patches to support it, but then we (or maybe it was just "me"?) decided that allowing lots of IDE disks would just be encouraging people to use the oldest, slowest type of disk in large configurations that would most benefit from going to the trouble of getting the proper virtio drivers installed in the guest.


So in the end, the qemu driver in libvirt only supports a single IDE controller, and only if it is built into the basic machinetype. That won't prevent any other driver from supporting multiple IDE controllers, but I would think twice about doing it.


Most cases there is a single IDE controller, with two channels,
and each channel has two devices, giving a total of 4 disks.

I'm just wondering if the code is mistakenly creating separate
libvirt controllers for the 2 IDE channels




[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