Re: [PATCH 03/12] hyperv: XML parsing of Ethernet adapters

2021-02-01 Thread Michal Privoznik

On 1/22/21 9:18 PM, Matt Coleman wrote:

Co-authored-by: Sri Ramanujam 
Signed-off-by: Matt Coleman 
---
  src/hyperv/hyperv_driver.c| 113 ++
  src/hyperv/hyperv_wmi.c   |  20 +
  src/hyperv/hyperv_wmi.h   |   8 ++
  src/hyperv/hyperv_wmi_classes.h   |  12 +++
  src/hyperv/hyperv_wmi_generator.input | 109 +
  5 files changed, 262 insertions(+)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 9394794c8f..ca75de98a9 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -1388,6 +1388,107 @@ hypervDomainDefParseSerial(virDomainDefPtr def, 
Msvm_ResourceAllocationSettingDa
  }
  
  
+static int

+hypervDomainDefParseEthernetAdapter(virDomainDefPtr def,
+Msvm_EthernetPortAllocationSettingData 
*net,
+hypervPrivate *priv)
+{
+virDomainNetDefPtr ndef = NULL;
+g_autoptr(Msvm_SyntheticEthernetPortSettingData) sepsd = NULL;
+g_autoptr(Msvm_VirtualEthernetSwitch) vSwitch = NULL;
+char **switchConnection = NULL;
+g_autofree char *switchConnectionEscaped = NULL;
+char *sepsdPATH = NULL;
+g_autofree char *sepsdEscaped = NULL;
+g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER;
+
+VIR_DEBUG("Parsing ethernet adapter '%s'", net->data->InstanceID);
+
+ndef = g_new0(virDomainNetDef, 1);
+
+ndef->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
+
+/*
+ * If there's no switch port connection or the EnabledState is disabled,
+ * then the adapter isn't hooked up to anything and we don't have to
+ * do anything more.
+ */
+switchConnection = net->data->HostResource.data;
+if (net->data->HostResource.count < 1 || !*switchConnection ||
+net->data->EnabledState == 
MSVM_ETHERNETPORTALLOCATIONSETTINGDATA_ENABLEDSTATE_DISABLED) {
+VIR_DEBUG("Adapter not connected to switch");


@ndef was allocated, but is leaked if this path is taken. Similarly, if 
any of errors below is hit. I think it's time to define g_auto() for 
virDomainNetDefPtr type nad have @ndef to be g_auto().


Or maybe simple reordering of some lines would be sufficient?


+return 0;
+}
+
+/*
+ * Now we retrieve the associated Msvm_SyntheticEthernetPortSettingData and
+ * Msvm_VirtualEthernetSwitch objects and use them to build the XML 
definition.
+ */
+
+/* begin by getting the Msvm_SyntheticEthernetPortSettingData object */
+sepsdPATH = net->data->Parent;
+sepsdEscaped = virStringReplace(sepsdPATH, "\\", "");
+virBufferAsprintf(,
+  MSVM_SYNTHETICETHERNETPORTSETTINGDATA_WQL_SELECT "WHERE 
__PATH = '%s'",
+  sepsdEscaped);
+
+if (hypervGetWmiClass(Msvm_SyntheticEthernetPortSettingData, ) < 0)
+return -1;
+
+if (!sepsd) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not retrieve NIC 
settings"));
+return -1;
+}
+
+/* set mac address */
+if (virMacAddrParseHex(sepsd->data->Address, >mac) < 0)
+return -1;
+
+/* now we get the Msvm_VirtualEthernetSwitch */
+virBufferFreeAndReset();
+switchConnectionEscaped = virStringReplace(*switchConnection, "\\", 
"");
+virBufferAsprintf(,
+  MSVM_VIRTUALETHERNETSWITCH_WQL_SELECT "WHERE __PATH = 
'%s'",
+  switchConnectionEscaped);
+
+if (hypervGetWmiClass(Msvm_VirtualEthernetSwitch, ) < 0)
+return -1;
+
+if (!vSwitch) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not retrieve virtual 
switch"));
+return -1;
+}
+
+/* get bridge name */
+ndef->data.bridge.brname = g_strdup(vSwitch->data->Name);
+
+if (VIR_APPEND_ELEMENT(def->nets, def->nnets, ndef) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not append definition 
to domain"));
+return -1;
+}
+
+return 0;
+}
+


Michal



[PATCH 03/12] hyperv: XML parsing of Ethernet adapters

2021-01-22 Thread Matt Coleman
Co-authored-by: Sri Ramanujam 
Signed-off-by: Matt Coleman 
---
 src/hyperv/hyperv_driver.c| 113 ++
 src/hyperv/hyperv_wmi.c   |  20 +
 src/hyperv/hyperv_wmi.h   |   8 ++
 src/hyperv/hyperv_wmi_classes.h   |  12 +++
 src/hyperv/hyperv_wmi_generator.input | 109 +
 5 files changed, 262 insertions(+)

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 9394794c8f..ca75de98a9 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -1388,6 +1388,107 @@ hypervDomainDefParseSerial(virDomainDefPtr def, 
Msvm_ResourceAllocationSettingDa
 }
 
 
+static int
+hypervDomainDefParseEthernetAdapter(virDomainDefPtr def,
+Msvm_EthernetPortAllocationSettingData 
*net,
+hypervPrivate *priv)
+{
+virDomainNetDefPtr ndef = NULL;
+g_autoptr(Msvm_SyntheticEthernetPortSettingData) sepsd = NULL;
+g_autoptr(Msvm_VirtualEthernetSwitch) vSwitch = NULL;
+char **switchConnection = NULL;
+g_autofree char *switchConnectionEscaped = NULL;
+char *sepsdPATH = NULL;
+g_autofree char *sepsdEscaped = NULL;
+g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER;
+
+VIR_DEBUG("Parsing ethernet adapter '%s'", net->data->InstanceID);
+
+ndef = g_new0(virDomainNetDef, 1);
+
+ndef->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
+
+/*
+ * If there's no switch port connection or the EnabledState is disabled,
+ * then the adapter isn't hooked up to anything and we don't have to
+ * do anything more.
+ */
+switchConnection = net->data->HostResource.data;
+if (net->data->HostResource.count < 1 || !*switchConnection ||
+net->data->EnabledState == 
MSVM_ETHERNETPORTALLOCATIONSETTINGDATA_ENABLEDSTATE_DISABLED) {
+VIR_DEBUG("Adapter not connected to switch");
+return 0;
+}
+
+/*
+ * Now we retrieve the associated Msvm_SyntheticEthernetPortSettingData and
+ * Msvm_VirtualEthernetSwitch objects and use them to build the XML 
definition.
+ */
+
+/* begin by getting the Msvm_SyntheticEthernetPortSettingData object */
+sepsdPATH = net->data->Parent;
+sepsdEscaped = virStringReplace(sepsdPATH, "\\", "");
+virBufferAsprintf(,
+  MSVM_SYNTHETICETHERNETPORTSETTINGDATA_WQL_SELECT "WHERE 
__PATH = '%s'",
+  sepsdEscaped);
+
+if (hypervGetWmiClass(Msvm_SyntheticEthernetPortSettingData, ) < 0)
+return -1;
+
+if (!sepsd) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not retrieve NIC 
settings"));
+return -1;
+}
+
+/* set mac address */
+if (virMacAddrParseHex(sepsd->data->Address, >mac) < 0)
+return -1;
+
+/* now we get the Msvm_VirtualEthernetSwitch */
+virBufferFreeAndReset();
+switchConnectionEscaped = virStringReplace(*switchConnection, "\\", 
"");
+virBufferAsprintf(,
+  MSVM_VIRTUALETHERNETSWITCH_WQL_SELECT "WHERE __PATH = 
'%s'",
+  switchConnectionEscaped);
+
+if (hypervGetWmiClass(Msvm_VirtualEthernetSwitch, ) < 0)
+return -1;
+
+if (!vSwitch) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not retrieve 
virtual switch"));
+return -1;
+}
+
+/* get bridge name */
+ndef->data.bridge.brname = g_strdup(vSwitch->data->Name);
+
+if (VIR_APPEND_ELEMENT(def->nets, def->nnets, ndef) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not append 
definition to domain"));
+return -1;
+}
+
+return 0;
+}
+
+static int
+hypervDomainDefParseEthernet(virDomainPtr domain,
+ virDomainDefPtr def,
+ Msvm_EthernetPortAllocationSettingData *nets)
+{
+Msvm_EthernetPortAllocationSettingData *entry = nets;
+hypervPrivate *priv = domain->conn->privateData;
+
+while (entry) {
+if (hypervDomainDefParseEthernetAdapter(def, entry, priv) < 0)
+return -1;
+
+entry = entry->next;
+}
+
+return 0;
+}
+
+
 /*
  * Driver functions
  */
@@ -2249,6 +2350,7 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int 
flags)
 g_autoptr(Msvm_StorageAllocationSettingData) sasd = NULL;
 g_autoptr(Msvm_SerialPortSettingData) spsd = NULL;
 Msvm_ResourceAllocationSettingData *serialDevices = NULL;
+g_autoptr(Msvm_EthernetPortAllocationSettingData) nets = NULL;
 
 virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS, NULL);
 
@@ -2290,6 +2392,10 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int 
flags)
 if (hypervGetSerialPortSD(priv, 
virtualSystemSettingData->data->InstanceID, ) < 0)
 return NULL;
 
+if (hypervGetEthernetPortAllocationSD(priv,
+  
virtualSystemSettingData->data->InstanceID, ) < 0)
+return NULL;
+
 /* Fill struct */
 def->virtType =