Re: [libvirt PATCH 2/2] esx: implement domainInterfaceAddresses

2020-09-14 Thread Pino Toscano
On Monday, 14 September 2020 17:33:02 CEST Michal Privoznik wrote:
> > +for (dynamicProperty = virtualMachine->propSet; dynamicProperty;
> > + dynamicProperty = dynamicProperty->_next) {
> > +if (STREQ(dynamicProperty->name, "guest.net")) {
> > +if (esxVI_GuestNicInfo_CastListFromAnyType
> > + (dynamicProperty->val, ) < 0) {
> > +goto cleanup;
> > +}
> > +}
> > +}
> > +
> > +if (!guestNicInfoList)
> > +goto cleanup;
> 
> This looks suspicious. If I understand the code correctly then this 
> means we haven't found any network config. What we usually do is we 
> return an empty array (*ifaces = NULL) and return 0. But if this is a 
> more serious error then we need a virReportError() here.

Good notice, it is actually an issue (we requested a property of a VM,
the SOAP call for it succeeded but there was no property in the
answer). I will amend and send v2.

Thanks,
-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.


Re: [libvirt PATCH 2/2] esx: implement domainInterfaceAddresses

2020-09-14 Thread Michal Privoznik

On 9/14/20 11:15 AM, Pino Toscano wrote:

Implement the .domainInterfaceAddresses hypervisor API, although only
functional for the VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT source.

Signed-off-by: Pino Toscano 
---
  docs/drvesx.html.in  |   4 ++
  src/esx/esx_driver.c | 166 +++
  2 files changed, 170 insertions(+)

diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in
index 38de640c2a..3acb7e5c51 100644
--- a/docs/drvesx.html.in
+++ b/docs/drvesx.html.in
@@ -799,6 +799,10 @@ Enter administrator password for example-vcenter.com:
  
  virDomainGetHostname
  
+
+virDomainInterfaceAddresses (only for the
+VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT source)
+
  
  virDomainReboot
  
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index bddc588977..0f63b5db4d 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -5113,6 +5113,171 @@ esxDomainGetHostname(virDomainPtr domain,
  }
  
  
+static int

+esxParseIPAddress(const char *ipAddress, int prefixLength,
+  virDomainIPAddress *addr)
+{
+virSocketAddr tmp_addr;
+virIPAddrType addr_type;
+
+if (virSocketAddrParseAny(_addr, ipAddress, AF_UNSPEC, false) <= 0)
+return 0;
+
+switch (VIR_SOCKET_ADDR_FAMILY(_addr)) {
+case AF_INET:
+addr_type = VIR_IP_ADDR_TYPE_IPV4;
+break;
+case AF_INET6:
+addr_type = VIR_IP_ADDR_TYPE_IPV6;
+break;
+default:
+return 0;
+}
+
+addr->type = addr_type;
+addr->addr = g_strdup(ipAddress);
+addr->prefix = prefixLength;
+
+return 1;
+}
+
+
+static int
+esxDomainInterfaceAddresses(virDomainPtr domain,
+virDomainInterfacePtr **ifaces,
+unsigned int source,
+unsigned int flags)
+{
+int result = -1;
+esxPrivate *priv = domain->conn->privateData;
+esxVI_String *propertyNameList = NULL;
+esxVI_ObjectContent *virtualMachine = NULL;
+esxVI_VirtualMachinePowerState powerState;
+esxVI_DynamicProperty *dynamicProperty;
+esxVI_GuestNicInfo *guestNicInfoList = NULL;
+esxVI_GuestNicInfo *guestNicInfo = NULL;
+virDomainInterfacePtr *ifaces_ret = NULL;
+size_t ifaces_count = 0;
+size_t i;
+int ret;
+
+virCheckFlags(0, -1);
+if (source != VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+   _("Unknown IP address data source %d"),
+   source);
+return -1;
+}
+
+if (esxVI_EnsureSession(priv->primary) < 0)
+return -1;
+
+if (esxVI_String_AppendValueListToList(,
+   "runtime.powerState\0"
+   "guest.net") < 0 ||
+esxVI_LookupVirtualMachineByUuid(priv->primary, domain->uuid,
+ propertyNameList, ,
+ esxVI_Occurrence_RequiredItem) ||
+esxVI_GetVirtualMachinePowerState(virtualMachine, ) < 0) {
+goto cleanup;
+}
+
+if (powerState != esxVI_VirtualMachinePowerState_PoweredOn) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("Domain is not powered on"));
+goto cleanup;
+}
+
+for (dynamicProperty = virtualMachine->propSet; dynamicProperty;
+ dynamicProperty = dynamicProperty->_next) {
+if (STREQ(dynamicProperty->name, "guest.net")) {
+if (esxVI_GuestNicInfo_CastListFromAnyType
+ (dynamicProperty->val, ) < 0) {
+goto cleanup;
+}
+}
+}
+
+if (!guestNicInfoList)
+goto cleanup;


This looks suspicious. If I understand the code correctly then this 
means we haven't found any network config. What we usually do is we 
return an empty array (*ifaces = NULL) and return 0. But if this is a 
more serious error then we need a virReportError() here.



+
+for (guestNicInfo = guestNicInfoList; guestNicInfo;
+ guestNicInfo = guestNicInfo->_next) {
+virDomainInterfacePtr iface = NULL;
+size_t addrs_count = 0;
+
+if (guestNicInfo->connected != esxVI_Boolean_True ||
+!guestNicInfo->network) {
+continue;
+}
+
+if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0)
+goto cleanup;
+
+if (VIR_ALLOC(ifaces_ret[ifaces_count - 1]) < 0)
+goto cleanup;
+
+iface = ifaces_ret[ifaces_count - 1];
+iface->naddrs = 0;
+iface->name = g_strdup(guestNicInfo->network);
+iface->hwaddr = g_strdup(guestNicInfo->macAddress);
+
+if (guestNicInfo->ipConfig) {
+esxVI_NetIpConfigInfoIpAddress *ipAddress;
+for (ipAddress = guestNicInfo->ipConfig->ipAddress; ipAddress;
+ ipAddress = 

[libvirt PATCH 2/2] esx: implement domainInterfaceAddresses

2020-09-14 Thread Pino Toscano
Implement the .domainInterfaceAddresses hypervisor API, although only
functional for the VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT source.

Signed-off-by: Pino Toscano 
---
 docs/drvesx.html.in  |   4 ++
 src/esx/esx_driver.c | 166 +++
 2 files changed, 170 insertions(+)

diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in
index 38de640c2a..3acb7e5c51 100644
--- a/docs/drvesx.html.in
+++ b/docs/drvesx.html.in
@@ -799,6 +799,10 @@ Enter administrator password for example-vcenter.com:
 
 virDomainGetHostname
 
+
+virDomainInterfaceAddresses (only for the
+VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT source)
+
 
 virDomainReboot
 
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index bddc588977..0f63b5db4d 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -5113,6 +5113,171 @@ esxDomainGetHostname(virDomainPtr domain,
 }
 
 
+static int
+esxParseIPAddress(const char *ipAddress, int prefixLength,
+  virDomainIPAddress *addr)
+{
+virSocketAddr tmp_addr;
+virIPAddrType addr_type;
+
+if (virSocketAddrParseAny(_addr, ipAddress, AF_UNSPEC, false) <= 0)
+return 0;
+
+switch (VIR_SOCKET_ADDR_FAMILY(_addr)) {
+case AF_INET:
+addr_type = VIR_IP_ADDR_TYPE_IPV4;
+break;
+case AF_INET6:
+addr_type = VIR_IP_ADDR_TYPE_IPV6;
+break;
+default:
+return 0;
+}
+
+addr->type = addr_type;
+addr->addr = g_strdup(ipAddress);
+addr->prefix = prefixLength;
+
+return 1;
+}
+
+
+static int
+esxDomainInterfaceAddresses(virDomainPtr domain,
+virDomainInterfacePtr **ifaces,
+unsigned int source,
+unsigned int flags)
+{
+int result = -1;
+esxPrivate *priv = domain->conn->privateData;
+esxVI_String *propertyNameList = NULL;
+esxVI_ObjectContent *virtualMachine = NULL;
+esxVI_VirtualMachinePowerState powerState;
+esxVI_DynamicProperty *dynamicProperty;
+esxVI_GuestNicInfo *guestNicInfoList = NULL;
+esxVI_GuestNicInfo *guestNicInfo = NULL;
+virDomainInterfacePtr *ifaces_ret = NULL;
+size_t ifaces_count = 0;
+size_t i;
+int ret;
+
+virCheckFlags(0, -1);
+if (source != VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+   _("Unknown IP address data source %d"),
+   source);
+return -1;
+}
+
+if (esxVI_EnsureSession(priv->primary) < 0)
+return -1;
+
+if (esxVI_String_AppendValueListToList(,
+   "runtime.powerState\0"
+   "guest.net") < 0 ||
+esxVI_LookupVirtualMachineByUuid(priv->primary, domain->uuid,
+ propertyNameList, ,
+ esxVI_Occurrence_RequiredItem) ||
+esxVI_GetVirtualMachinePowerState(virtualMachine, ) < 0) {
+goto cleanup;
+}
+
+if (powerState != esxVI_VirtualMachinePowerState_PoweredOn) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("Domain is not powered on"));
+goto cleanup;
+}
+
+for (dynamicProperty = virtualMachine->propSet; dynamicProperty;
+ dynamicProperty = dynamicProperty->_next) {
+if (STREQ(dynamicProperty->name, "guest.net")) {
+if (esxVI_GuestNicInfo_CastListFromAnyType
+ (dynamicProperty->val, ) < 0) {
+goto cleanup;
+}
+}
+}
+
+if (!guestNicInfoList)
+goto cleanup;
+
+for (guestNicInfo = guestNicInfoList; guestNicInfo;
+ guestNicInfo = guestNicInfo->_next) {
+virDomainInterfacePtr iface = NULL;
+size_t addrs_count = 0;
+
+if (guestNicInfo->connected != esxVI_Boolean_True ||
+!guestNicInfo->network) {
+continue;
+}
+
+if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0)
+goto cleanup;
+
+if (VIR_ALLOC(ifaces_ret[ifaces_count - 1]) < 0)
+goto cleanup;
+
+iface = ifaces_ret[ifaces_count - 1];
+iface->naddrs = 0;
+iface->name = g_strdup(guestNicInfo->network);
+iface->hwaddr = g_strdup(guestNicInfo->macAddress);
+
+if (guestNicInfo->ipConfig) {
+esxVI_NetIpConfigInfoIpAddress *ipAddress;
+for (ipAddress = guestNicInfo->ipConfig->ipAddress; ipAddress;
+ ipAddress = ipAddress->_next) {
+virDomainIPAddress ip_addr;
+
+ret = esxParseIPAddress(ipAddress->ipAddress,
+ipAddress->prefixLength->value, 
_addr);
+if (ret < 0)
+goto cleanup;
+else if (ret == 0)
+