Re: [libvirt PATCH 6/6] tools: report messages for 'dominfo' command

2021-02-05 Thread Pino Toscano
On Friday, 5 February 2021 15:18:31 CET Daniel P. Berrangé wrote:
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 02ff1fbd62..df24543ef8 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -1291,6 +1291,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
>  char *str, uuid[VIR_UUID_STRING_BUFLEN];
>  int has_managed_save = 0;
>  virshControlPtr priv = ctl->privData;
> +char **messages = NULL;
>  
>  if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
>  return false;
> @@ -1391,6 +1392,18 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
>  VIR_FREE(seclabel);
>  }
>  }
> +
> +if (virDomainGetMessages(dom, , 0) > 0) {
> +size_t i;
> +for (i = 0; messages[i] != NULL; i++) {
> +if (i == 0) {
> +vshPrint(ctl, "%-15s %s\n", _("Messages:"), messages[i]);
> +} else {
> +vshPrint(ctl, "%-15s %s\n", "", messages[i]);
> +}
> +}
> +}

'messages' is leaked here.

-- 
Pino Toscano

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


Re: [libvirt PATCH 5/6] qemu: implement virDomainGetMessages API

2021-02-05 Thread Pino Toscano
On Friday, 5 February 2021 15:18:30 CET Daniel P. Berrangé wrote:
> +for (i = 0; i < VIR_DOMAIN_TAINT_LAST; i++) {
> +if (vm->taint & (1 << i)) {
> +(*msgs)[n++] = g_strdup_printf(
> +"%s: %s", _("tainted"),

Please translate the whole string, instead of composing a string
puzzle, e.g.: g_strdup_printf(_("tainted: %s"), msg)

There are languages that may need to tweak capitalization and/or
punctuation of text; for example, in French a space is needed
before and after : (colon).

> +if (!flags || (flags & VIR_DOMAIN_MESSAGE_DEPRECATION)) {
> +nmsgs += vm->ndeprecations;
> +*msgs = g_renew(char *, *msgs, nmsgs+1);
> +
> +for (i = 0; i < vm->ndeprecations; i++) {
> +(*msgs)[n++] = g_strdup_printf(
> +"%s: %s",
> +    _("deprecated configuration"),
> +vm->deprecations[i]);

Ditto.

-- 
Pino Toscano

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


Re: [PATCH v2 15/18] tests: vmx: Add element for all domain XMLs

2020-10-19 Thread Pino Toscano
On Monday, 19 October 2020 14:22:24 CEST Peter Krempa wrote:
>  is mandatory for a domain XML. Add 'displayName' for all the test
> cases which were missing them so that  is parsed correctly.
> 
> Signed-off-by: Peter Krempa 
> ---
> [...]
> diff --git a/tests/vmx2xmldata/vmx2xml-case-insensitive-1.vmx 
> b/tests/vmx2xmldata/vmx2xml-case-insensitive-1.vmx
> index 8641c5c01b..dd069b378d 100644
> --- a/tests/vmx2xmldata/vmx2xml-case-insensitive-1.vmx
> +++ b/tests/vmx2xmldata/vmx2xml-case-insensitive-1.vmx
> @@ -49,3 +49,4 @@ UUID.LOCATION = "56 4D B5 06 A2 BD FB EB-AE 86 F7 D8 49 27 
> D0 C4"
>  SCHED.CPU.MAX = "UNLIMITED"
>  SCHED.SWAP.DERIVEDNAME = 
> "/VMFS/VOLUMES/498076B2-02796C1A-EF5B-000AE484A6A3/FEDORA11/FEDORA11-7DE040D8.VSWP"
>  TOOLS.REMINDINSTALL = "TRUE"
> +displayName = "test"
> diff --git a/tests/vmx2xmldata/vmx2xml-case-insensitive-2.vmx 
> b/tests/vmx2xmldata/vmx2xml-case-insensitive-2.vmx
> index a485d03cd9..e37e0b9369 100644
> --- a/tests/vmx2xmldata/vmx2xml-case-insensitive-2.vmx
> +++ b/tests/vmx2xmldata/vmx2xml-case-insensitive-2.vmx
> @@ -49,3 +49,4 @@ uuid.location = "56 4d b5 06 a2 bd fb eb-ae 86 f7 d8 49 27 
> d0 c4"
>  sched.cpu.max = "unlimited"
>  sched.swap.derivedname = 
> "/vmfs/volumes/498076b2-02796c1a-ef5b-000ae484a6a3/fedora11/fedora11-7de040d8.vswp"
>  tools.remindinstall = "true"
> +displayName = "test"

Are you sure about these two? They have already displayName, albeit
with a different captalization, and indeed this patch has no changes
to their xml result files.

With the above two removed:
Reviewed-by: Pino Toscano 

-- 
Pino Toscano

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


Re: [PATCH v2 1/7] hyperv: implement domainSetAutostart

2020-10-14 Thread Pino Toscano
On Tuesday, 13 October 2020 07:13:58 CEST Matt Coleman wrote:
> Co-authored-by: Sri Ramanujam 
> Signed-off-by: Matt Coleman 
> ---
>  src/hyperv/hyperv_driver.c | 91 ++
>  1 file changed, 91 insertions(+)
> 
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index 2ac30fa4c6..79b48a9dff 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -1310,6 +1310,96 @@ hypervDomainGetAutostart(virDomainPtr domain, int 
> *autostart)
>  
>  
>  
> +static int
> +hypervDomainSetAutostart(virDomainPtr domain, int autostart)
> +{
> +int result = -1;
> +char uuid_string[VIR_UUID_STRING_BUFLEN];
> +hypervPrivate *priv = domain->conn->privateData;
> +Msvm_VirtualSystemSettingData *vssd = NULL;
> +hypervInvokeParamsListPtr params = NULL;
> +g_auto(virBuffer) eprQuery = VIR_BUFFER_INITIALIZER;
> +virHashTablePtr autostartParam = NULL;
> +hypervWmiClassInfoListPtr embeddedParamClass = NULL;
> +const char *methodName = NULL, *embeddedParamName = NULL;
> +char enabledValue[] = "2", disabledValue[] = "0";

Not that it is an issue: IMHO using for enabledValue/disabledValue the
same approach as methodName/embeddedParamName, i.e. simple const char*
ponting to static strings, is easier.

Also there is a bit of style mixup:
- methodName/embeddedParamName/etc are NULL by default, set to a value
  for V1 or V2 (left NULL otherwise)
- enabledValue/disabledValue are set to the values for V1, and changed
  to V2
- other patches in this series (#5 and #6) also do "V1 by default,
  change for V2"
IMHO a single style would make things easier, especially in case there
will ever be a V3.

> +if (hypervInvokeMethod(priv, params, NULL) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +       _("Could not set autostart"));

Minor question: is there really no way to get the reason (in form of
string) of the failure?

-- 
Pino Toscano

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


Re: [PATCH v2 3/7] hyperv: implement domainReboot and domainReset

2020-10-14 Thread Pino Toscano
On Wednesday, 14 October 2020 05:23:21 CEST Matt Coleman wrote:
> > Note that virDomainReboot() can take various flags, however none is
> > used ATM.
> 
> Would it be okay to stick with what I’ve got, now, and look at adding 
> support for the flags in a future commit?

Sorry, I did not express myself properly: I did not want to say you
have to add support for these flags, but merely that the functions
implemented by this patch do not. Hence, depending on what you plan to
add to them (or even whether any of the flags can be supported), my
earlier suggestion of a common helper might make sense or not.

-- 
Pino Toscano

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


Re: [PATCH v2 4/7] hyperv: implement domainShutdown and domainShutdownFlags

2020-10-13 Thread Pino Toscano
On Tuesday, 13 October 2020 07:14:01 CEST Matt Coleman wrote:
> +hypervAddSimpleParam(params, "Force", "False");
> +hypervAddSimpleParam(params, "Reason", _("Planned shutdown via 
> libvirt"));

Is this message logged in the Hyper-V logs? If so, then I'd not
translate it: imagine an user running libvirt on an e.g. a French
locale, sending this query to an Hyper-V on e.g. German. While English
is not that different in this situation (still potentially not the
Hyper-V language), at least IMHO it gives something understandable.

Of course, in case the message is changed to an untranslated text,
then please leave a comment there, so noone accidentally makes it
translatable.

-- 
Pino Toscano

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


Re: [PATCH v2 3/7] hyperv: implement domainReboot and domainReset

2020-10-13 Thread Pino Toscano
On Tuesday, 13 October 2020 07:14:00 CEST Matt Coleman wrote:
> Co-authored-by: Sri Ramanujam 
> Signed-off-by: Matt Coleman 
> ---
>  src/hyperv/hyperv_driver.c  | 48 +
>  src/hyperv/hyperv_wmi_classes.h |  1 +
>  2 files changed, 49 insertions(+)
> 
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index c4fca4685e..7182340f74 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -917,6 +917,52 @@ hypervDomainResume(virDomainPtr domain)
>  
>  
>  
> +static int
> +hypervDomainReboot(virDomainPtr domain, unsigned int flags)
> +{
> +int result = -1;
> +hypervPrivate *priv = domain->conn->privateData;
> +Msvm_ComputerSystem *computerSystem = NULL;
> +
> +virCheckFlags(0, -1);
> +
> +if (hypervMsvmComputerSystemFromDomain(domain, ) < 0)
> +goto cleanup;
> +
> +result = hypervInvokeMsvmComputerSystemRequestStateChange(domain,
> +
> MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_REBOOT);
> +
> + cleanup:
> +hypervFreeObject(priv, (hypervObject *)computerSystem);
> +
> +return result;
> +}
> +
> +
> +
> +static int
> +hypervDomainReset(virDomainPtr domain, unsigned int flags)
> +{
> +int result = -1;
> +hypervPrivate *priv = domain->conn->privateData;
> +Msvm_ComputerSystem *computerSystem = NULL;
> +
> +virCheckFlags(0, -1);
> +
> +if (hypervMsvmComputerSystemFromDomain(domain, ) < 0)
> +goto cleanup;
> +
> +result = hypervInvokeMsvmComputerSystemRequestStateChange(domain,
> +
> MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_RESET);
> +
> + cleanup:
> +hypervFreeObject(priv, (hypervObject *)computerSystem);
> +
> +return result;
> +}

What about making a common helper function that calls
hypervMsvmComputerSystemFromDomain +
hypervInvokeMsvmComputerSystemRequestStateChange?

Note that virDomainReboot() can take various flags, however none is
used ATM.

-- 
Pino Toscano

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


[libvirt PATCH 3/4] vmx: expand the disk array

2020-10-12 Thread Pino Toscano
Account for the possible SATA disks too, which means 120 potential
disks.

This means the size of the array triples, however that is unavoidable
with the current way of reading disks.

Signed-off-by: Pino Toscano 
---
 src/vmx/vmx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 0ec6222f50..6e4b455794 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -1645,8 +1645,8 @@ virVMXParseConfig(virVMXContext *ctx,
 if (def->graphics[def->ngraphics] != NULL)
 ++def->ngraphics;
 
-/* def:disks: 4 * 15 scsi + 2 * 2 ide + 2 floppy = 66 */
-def->disks = g_new0(virDomainDiskDefPtr, 66);
+/* def:disks: 4 * 15 scsi + 4 * 30 sata + 2 * 2 ide + 2 floppy = 186 */
+def->disks = g_new0(virDomainDiskDefPtr, 186);
 def->ndisks = 0;
 
 /* def:disks (scsi) */
-- 
2.26.2



[libvirt PATCH 0/4] vmx: start parsing SATA disks

2020-10-12 Thread Pino Toscano
Try to parse SATA disks in VMware guests from their configs in VMX
files. There are a couple of helper/cleanup commits to ease a bit the
actual patches.

Pino Toscano (4):
  vmx: hide private helpers
  vmx: shortcut 'cdrom-image' as CD-ROM earlier
  vmx: expand the disk array
  vmx: start parsing SATA disks

 src/libvirt_vmx.syms  |  12 -
 src/vmx/vmx.c | 212 --
 src/vmx/vmx.h |  44 
 .../vmx2xml-esx-in-the-wild-10.vmx| 101 +
 .../vmx2xml-esx-in-the-wild-10.xml|  36 +++
 .../vmx2xmldata/vmx2xml-esx-in-the-wild-8.xml |   6 +
 tests/vmx2xmltest.c   |   1 +
 7 files changed, 335 insertions(+), 77 deletions(-)
 create mode 100644 tests/vmx2xmldata/vmx2xml-esx-in-the-wild-10.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-esx-in-the-wild-10.xml

-- 
2.26.2



[libvirt PATCH 1/4] vmx: hide private helpers

2020-10-12 Thread Pino Toscano
Move all the private helpers for parsing and formatting of domain
elements as private static functions in vmx.c, to avoid using them
directly.

Signed-off-by: Pino Toscano 
---
 src/libvirt_vmx.syms | 12 -
 src/vmx/vmx.c| 62 
 src/vmx/vmx.h| 44 ---
 3 files changed, 46 insertions(+), 72 deletions(-)

diff --git a/src/libvirt_vmx.syms b/src/libvirt_vmx.syms
index e673923bd9..9d723c6de4 100644
--- a/src/libvirt_vmx.syms
+++ b/src/libvirt_vmx.syms
@@ -7,19 +7,7 @@ virVMXConvertToUTF8;
 virVMXDomainXMLConfInit;
 virVMXEscapeHex;
 virVMXFormatConfig;
-virVMXFormatDisk;
-virVMXFormatEthernet;
-virVMXFormatFloppy;
-virVMXFormatParallel;
-virVMXFormatSerial;
-virVMXFormatVNC;
 virVMXParseConfig;
-virVMXParseDisk;
-virVMXParseEthernet;
-virVMXParseParallel;
-virVMXParseSCSIController;
-virVMXParseSerial;
-virVMXParseVNC;
 virVMXUnescapeHex;
 
 # Let emacs know we want case-insensitive sorting
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index e0777a9ddd..70f4d78e8a 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -521,6 +521,35 @@ VIR_ENUM_IMPL(virVMXControllerModelSCSI,
   "UNUSED virtio-non-transitional",
 );
 
+static int virVMXParseVNC(virConfPtr conf, virDomainGraphicsDefPtr *def);
+static int virVMXParseSCSIController(virConfPtr conf, int controller, bool 
*present,
+ int *virtualDev);
+static int virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt,
+   virConfPtr conf, int device, int busType,
+   int controllerOrBus, int unit, virDomainDiskDefPtr 
*def,
+   virDomainDefPtr vmdef);
+static int virVMXParseFileSystem(virConfPtr conf, int number, 
virDomainFSDefPtr *def);
+static int virVMXParseEthernet(virConfPtr conf, int controller, 
virDomainNetDefPtr *def);
+static int virVMXParseSerial(virVMXContext *ctx, virConfPtr conf, int port,
+ virDomainChrDefPtr *def);
+static int virVMXParseParallel(virVMXContext *ctx, virConfPtr conf, int port,
+   virDomainChrDefPtr *def);
+static int virVMXParseSVGA(virConfPtr conf, virDomainVideoDefPtr *def);
+
+static int virVMXFormatVNC(virDomainGraphicsDefPtr def, virBufferPtr buffer);
+static int virVMXFormatDisk(virVMXContext *ctx, virDomainDiskDefPtr def,
+   virBufferPtr buffer);
+static int virVMXFormatFloppy(virVMXContext *ctx, virDomainDiskDefPtr def,
+  virBufferPtr buffer, bool floppy_present[2]);
+static int virVMXFormatFileSystem(virDomainFSDefPtr def, int number,
+  virBufferPtr buffer);
+static int virVMXFormatEthernet(virDomainNetDefPtr def, int controller,
+virBufferPtr buffer, int virtualHW_version);
+static int virVMXFormatSerial(virVMXContext *ctx, virDomainChrDefPtr def,
+  virBufferPtr buffer);
+static int virVMXFormatParallel(virVMXContext *ctx, virDomainChrDefPtr def,
+virBufferPtr buffer);
+static int virVMXFormatSVGA(virDomainVideoDefPtr def, virBufferPtr buffer);
 
 /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
  * Helpers
@@ -1852,7 +1881,7 @@ virVMXParseConfig(virVMXContext *ctx,
 
 
 
-int
+static int
 virVMXParseVNC(virConfPtr conf, virDomainGraphicsDefPtr *def)
 {
 bool enabled = false;
@@ -1916,7 +1945,7 @@ virVMXParseVNC(virConfPtr conf, virDomainGraphicsDefPtr 
*def)
 
 
 
-int
+static int
 virVMXParseSCSIController(virConfPtr conf, int controller, bool *present,
   int *virtualDev)
 {
@@ -1986,7 +2015,7 @@ virVMXParseSCSIController(virConfPtr conf, int 
controller, bool *present,
 
 
 
-int
+static int
 virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr 
conf,
 int device, int busType, int controllerOrBus, int unit,
 virDomainDiskDefPtr *def, virDomainDefPtr vmdef)
@@ -2406,7 +2435,8 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 
 
 
-int virVMXParseFileSystem(virConfPtr conf, int number, virDomainFSDefPtr *def)
+static int
+virVMXParseFileSystem(virConfPtr conf, int number, virDomainFSDefPtr *def)
 {
 int result = -1;
 char prefix[48] = "";
@@ -2493,7 +2523,7 @@ int virVMXParseFileSystem(virConfPtr conf, int number, 
virDomainFSDefPtr *def)
 
 
 
-int
+static int
 virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def)
 {
 int result = -1;
@@ -2723,7 +2753,7 @@ virVMXParseEthernet(virConfPtr conf, int controller, 
virDomainNetDefPtr *def)
 
 
 
-int
+static int
 virVMXParseSerial(virVMXContext *ctx, virConfPtr conf, int port,
   virDomainChrDefPtr *def)
 {
@@ -2905,7 +2935,7 @@ virVMXParseSerial(virVMXContext *ctx, virConfPtr conf, 
int port,
 

[libvirt PATCH 2/4] vmx: shortcut 'cdrom-image' as CD-ROM earlier

2020-10-12 Thread Pino Toscano
Add it to the list of 'deviceType' values ignored for disks.

Signed-off-by: Pino Toscano 
---
 src/vmx/vmx.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 70f4d78e8a..0ec6222f50 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2235,13 +2235,15 @@ virVMXParseDisk(virVMXContext *ctx, 
virDomainXMLOptionPtr xmlopt, virConfPtr con
 (deviceType &&
  (STRCASEEQ(deviceType, "atapi-cdrom") ||
   STRCASEEQ(deviceType, "cdrom-raw") ||
+  STRCASEEQ(deviceType, "cdrom-image") ||
   (STRCASEEQ(deviceType, "scsi-passthru") &&
STRPREFIX(fileName, "/vmfs/devices/cdrom/") {
 /*
  * This function was called in order to parse a harddisk device,
- * but .iso files, 'atapi-cdrom', 'cdrom-raw', and 'scsi-passthru'
- * CDROM devices are for CDROM devices only. Just ignore it, 
another
- * call to this function to parse a CDROM device may handle it.
+ * but .iso files, 'atapi-cdrom', 'cdrom-raw', 'cdrom-image',
+ * and 'scsi-passthru' CDROM devices are for CDROM devices only.
+ * Just ignore it, another call to this function to parse a CDROM
+ * device may handle it.
  */
 goto ignore;
 } else if (virStringHasCaseSuffix(fileName, ".vmdk")) {
-- 
2.26.2



[libvirt PATCH 4/4] vmx: start parsing SATA disks

2020-10-12 Thread Pino Toscano
Always reverse-engineering VMX files, attempt to support SATA disks in
guests, and their controllers.

The esx-in-the-wild-10 test case is taken from RHBZ#1883588, while the
result of esx-in-the-wild-8 is updated with SATA disks.

Fixes (hopefully):
https://bugzilla.redhat.com/show_bug.cgi?id=1677608
https://bugzilla.redhat.com/show_bug.cgi?id=1883588

Signed-off-by: Pino Toscano 
---
 src/vmx/vmx.c | 138 ++
 .../vmx2xml-esx-in-the-wild-10.vmx| 101 +
 .../vmx2xml-esx-in-the-wild-10.xml|  36 +
 .../vmx2xmldata/vmx2xml-esx-in-the-wild-8.xml |   6 +
 tests/vmx2xmltest.c   |   1 +
 5 files changed, 282 insertions(+)
 create mode 100644 tests/vmx2xmldata/vmx2xml-esx-in-the-wild-10.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-esx-in-the-wild-10.xml

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 6e4b455794..51d88de750 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -90,6 +90,7 @@ def->os
 ## disks 
###
 
 scsi[0..3]:[0..6,8..15] -> 
: with 1 bus per controller
+sata[0..3]:[0..29] -> 
: with 1 bus per controller
 ide[0..1]:[0..1]-> 
: with 1 controller
 floppy[0..1]->  with 
1 controller and 1 bus per controller
 
@@ -120,6 +121,26 @@ def->disks[0]...
 ->slotnum
 
 
+## disks: sata hard drive from .vmdk image 
#
+
+sata0.present = "true" 
 # defaults to "false"
+sata0:0.present = "true"   
 # defaults to "false"
+sata0:0.startConnected = "true"
 # defaults to "true"
+
+...
+->type = _DISK_TYPE_FILE  <=>   sata0:0.deviceType = "???" 
 # defaults to ?
+->device = _DISK_DEVICE_DISK  <=>   sata0:0.deviceType = "???" 
 # defaults to ?
+->bus = _DISK_BUS_SATA
+->src = .vmdk  <=>   sata0:0.fileName = ".vmdk"
+->dst = sd[ * 30 +  mapped to [a-z]+]
+->driverName =<=>   sata0.virtualDev = ""  
 # default depends on guestOS value
+->driverType
+->cachemode   <=>   sata0:0.writeThrough = ""   
 # defaults to false, true -> _DISK_CACHE_WRITETHRU, false _DISK_CACHE_DEFAULT
+->readonly
+->shared
+->slotnum
+
+
 ## disks: ide hard drive from .vmdk image 
##
 
 ide0:0.present = "true"
 # defaults to "false"
@@ -164,6 +185,26 @@ def->disks[0]...
 ->slotnum
 
 
+## disks: sata cdrom from .iso image 
###
+
+sata0.present = "true" 
 # defaults to "false"
+sata0:0.present = "true"   
 # defaults to "false"
+sata0:0.startConnected = "true"
 # defaults to "true"
+
+...
+->type = _DISK_TYPE_FILE  <=>   sata0:0.deviceType = "cdrom-image" 
 # defaults to ?
+->device = _DISK_DEVICE_CDROM <=>   sata0:0.deviceType = "cdrom-image" 
 # defaults to ?
+->bus = _DISK_BUS_SATA
+->src = .iso   <=>   sata0:0.fileName = ".iso"
+->dst = sd[ * 30 +  mapped to [a-z]+]
+->driverName =<=>   sata0.virtualDev = ""  
 # default depends on guestOS value
+->driverType
+->cachemode
+->readonly
+->shared
+->slotnum
+
+
 ## disks: ide cdrom from .iso image 

 
 ide0:0.present = "true"
 # defaults to "false"
@@ -524,6 +565,7 @@ VIR_ENUM_IMPL(virVMXControllerModelSCSI,
 static int virVMXParseVNC(virConfPtr conf, virDomainGraphicsDefPtr *def);
 static int virVMXParseSCSIController(virConfPtr conf, int controller, bool 
*present,
  int *virtualDev);
+static int virVMXParseSATAController(virConfPtr conf, int controller, bool 
*present);
 static int virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt,
virConfPtr conf, int device, int busType,
int controllerOrBus, int unit, virDomainDiskDefPtr 
*def,
@@ -1335,6 +1377,7 @@ virVMXParseConfig(virVMXContext *ctx,
 long long coresPerSocket = 0;
 vir

Re: [PATCH 1/7] hyperv: implement domainSetAutostart

2020-10-09 Thread Pino Toscano
On Friday, 9 October 2020 10:31:50 CEST Matt Coleman wrote:
> +static int
> +hypervDomainSetAutostart(virDomainPtr domain, int autostart)
> +{
> +int result = -1;
> +char uuid_string[VIR_UUID_STRING_BUFLEN];
> +hypervPrivate *priv = domain->conn->privateData;
> +Msvm_VirtualSystemSettingData *vssd = NULL;
> +hypervInvokeParamsListPtr params = NULL;
> +g_auto(virBuffer) eprQuery = VIR_BUFFER_INITIALIZER;
> +virHashTablePtr autostartParam = NULL;
> +hypervWmiClassInfoListPtr embeddedParamClass = NULL;
> +const char *methodName = NULL, *embeddedParamName = NULL;
> +g_autofree char *enabledValue = NULL, *disabledValue = NULL;
> +
> +if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) {
> +methodName = "ModifyVirtualSystem";
> +embeddedParamName = "SystemSettingData";
> +embeddedParamClass = Msvm_VirtualSystemGlobalSettingData_WmiInfo;
> +enabledValue = g_strdup("2");
> +disabledValue = g_strdup("0");
> +} else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2) {
> +methodName = "ModifySystemSettings";
> +embeddedParamName = "SystemSettings";
> +embeddedParamClass = Msvm_VirtualSystemSettingData_WmiInfo;
> +enabledValue = g_strdup("4");
> +disabledValue = g_strdup("2");
> +}

It looks like 'enabledValue' and 'disabledValue' can be static strings
(like 'methodName' and 'embeddedParamName').

-- 
Pino Toscano

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


Re: [PATCH v2 18/21] schema: domain: Accept VMWARE disk sources for the disk

2020-10-08 Thread Pino Toscano
On Thursday, 8 October 2020 10:47:16 CEST Peter Krempa wrote:
> Our vmware driver chose to format disk paths starting with a square
> bracket. This would not conform to the RNG schema for disk source.
> Modify the schema to allow these since it's around for some time.

s/VMWARE/VMware/ in the first line of the commit message.

Also, the format is not a real path: it's the datastore where the VM
is stored, followed by the relative path in it. This is also
documented: https://libvirt.org/drvesx.html#datastore

> diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
> index b1fb939aff..42e94c40e9 100644
> --- a/docs/schemas/basictypes.rng
> +++ b/docs/schemas/basictypes.rng
> @@ -303,6 +303,12 @@
>  
>
> 
> +  
> +
> +  \[.+\] .+

Let's make the datastore part slightly more strict:

 \[[^\]+\] .+

So it should work also with a greedy validator.

With the above changes:

Reviewed-by: Pino Toscano 

-- 
Pino Toscano

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


Re: [PATCH 18/21] tests: vmx: Make paths in test files conform with the XML schema

2020-10-08 Thread Pino Toscano
On Thursday, 8 October 2020 09:55:32 CEST Peter Krempa wrote:
> The vmx tests use fake paths for files which in some cases didn't start
> with a /. Since libvirt's schema mandates full paths and the
> representation used in the tests is only internal to the tests we'll add
> a leading / to the paths to conform to the schema.
> 
> Signed-off-by: Peter Krempa 
> ---
>  tests/vmx2xmldata/vmx2xml-case-insensitive-1.xml   | 2 +-
>  tests/vmx2xmldata/vmx2xml-case-insensitive-2.xml   | 2 +-
>  tests/vmx2xmldata/vmx2xml-cdrom-ide-file.xml   | 2 +-
>  tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml  | 2 +-
>  tests/vmx2xmldata/vmx2xml-esx-in-the-wild-1.xml| 2 +-
>  tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml| 6 +++---
>  tests/vmx2xmldata/vmx2xml-esx-in-the-wild-3.xml| 6 +++---
>  tests/vmx2xmldata/vmx2xml-esx-in-the-wild-4.xml| 8 
>  tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.xml| 4 ++--
>  tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml| 2 +-
>  tests/vmx2xmldata/vmx2xml-esx-in-the-wild-7.xml| 2 +-
>  tests/vmx2xmldata/vmx2xml-esx-in-the-wild-8.xml| 6 +++---
>  tests/vmx2xmldata/vmx2xml-esx-in-the-wild-9.xml| 2 +-
>  tests/vmx2xmldata/vmx2xml-floppy-file.xml  | 4 ++--
>  tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml | 2 +-
>  tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-1.xml| 2 +-
>  tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-2.xml| 2 +-
>  tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-3.xml| 2 +-
>  tests/vmx2xmldata/vmx2xml-gsx-in-the-wild-4.xml| 2 +-
>  tests/vmx2xmldata/vmx2xml-harddisk-ide-file.xml| 2 +-
>  tests/vmx2xmldata/vmx2xml-harddisk-scsi-file.xml   | 2 +-
>  tests/vmx2xmldata/vmx2xml-harddisk-transient.xml   | 2 +-
>  tests/vmx2xmldata/vmx2xml-parallel-file.xml| 2 +-
>  tests/vmx2xmldata/vmx2xml-scsi-driver.xml  | 8 
>  tests/vmx2xmldata/vmx2xml-scsi-writethrough.xml| 2 +-
>  tests/vmx2xmldata/vmx2xml-serial-file.xml  | 4 ++--
>  tests/vmx2xmltest.c| 4 ++--
>  tests/xml2vmxdata/xml2vmx-cdrom-ide-file.xml   | 2 +-
>  tests/xml2vmxdata/xml2vmx-cdrom-scsi-file.xml  | 2 +-
>  tests/xml2vmxdata/xml2vmx-esx-in-the-wild-1.xml| 2 +-
>  tests/xml2vmxdata/xml2vmx-esx-in-the-wild-2.xml| 6 +++---
>  tests/xml2vmxdata/xml2vmx-esx-in-the-wild-3.xml| 6 +++---
>  tests/xml2vmxdata/xml2vmx-esx-in-the-wild-4.xml| 8 
>  tests/xml2vmxdata/xml2vmx-esx-in-the-wild-5.xml| 4 ++--
>  tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.xml| 2 +-
>  tests/xml2vmxdata/xml2vmx-esx-in-the-wild-7.xml| 2 +-
>  tests/xml2vmxdata/xml2vmx-esx-in-the-wild-9.xml| 2 +-
>  tests/xml2vmxdata/xml2vmx-floppy-file.xml  | 4 ++--
>  tests/xml2vmxdata/xml2vmx-fusion-in-the-wild-1.xml | 2 +-
>  tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-1.xml| 2 +-
>  tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-2.xml| 2 +-
>  tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-3.xml| 2 +-
>  tests/xml2vmxdata/xml2vmx-gsx-in-the-wild-4.xml| 2 +-
>  tests/xml2vmxdata/xml2vmx-harddisk-ide-file.xml| 2 +-
>  tests/xml2vmxdata/xml2vmx-harddisk-scsi-file.xml   | 2 +-
>  tests/xml2vmxdata/xml2vmx-parallel-file.xml| 2 +-
>  tests/xml2vmxdata/xml2vmx-scsi-driver.xml  | 8 
>  tests/xml2vmxdata/xml2vmx-scsi-writethrough.xml| 2 +-
>  tests/xml2vmxdata/xml2vmx-serial-file.xml  | 2 +-
>  tests/xml2vmxtest.c| 4 ++--
>  50 files changed, 79 insertions(+), 79 deletions(-)
> 
> diff --git a/tests/vmx2xmldata/vmx2xml-case-insensitive-1.xml 
> b/tests/vmx2xmldata/vmx2xml-case-insensitive-1.xml
> index 7cb6413941..97fa300c18 100644
> --- a/tests/vmx2xmldata/vmx2xml-case-insensitive-1.xml
> +++ b/tests/vmx2xmldata/vmx2xml-case-insensitive-1.xml
> @@ -16,7 +16,7 @@
>destroy
>
>  
> -  
> +  

The real XMLs of VMware guests have disks paths like
'[datastore] path/to/file.vmx'. The proposed change will make these
XMLs unusable as real XMLs, and some of them were actually real XMLs.

Even if these files are just tests, I don't think this is correct to
make them different than real XMLs.

If the schema does not support file='[datastore] path/to/file.vmx',
IMHO that's the schema itself what needs to be fixed, not example/test
XMLs...

-- 
Pino Toscano

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


[libvirt PATCH 2/2] esx: switch esxUtil_ResolveHostname to return a new string

2020-10-05 Thread Pino Toscano
Change the interface of esxUtil_ResolveHostname() to return a newly
allocated string with the result address, instead of forcing the callers
to provide a buffer and its size. This way we can simply (auto)free the
string, and make the function stacks smaller.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_driver.c | 20 +++-
 src/esx/esx_util.c   | 11 +++
 src/esx/esx_util.h   |  3 +--
 3 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index e82e5ed835..a17bf58a51 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -602,7 +602,7 @@ esxConnectToHost(esxPrivate *priv,
  char **vCenterIPAddress)
 {
 int result = -1;
-char ipAddress[NI_MAXHOST] = "";
+g_autofree char *ipAddress = NULL;
 char *username = NULL;
 char *password = NULL;
 char *url = NULL;
@@ -615,7 +615,7 @@ esxConnectToHost(esxPrivate *priv,
 
 ESX_VI_CHECK_ARG_LIST(vCenterIPAddress);
 
-if (esxUtil_ResolveHostname(conn->uri->server, ipAddress, NI_MAXHOST) < 0)
+if (esxUtil_ResolveHostname(conn->uri->server, ) < 0)
 return -1;
 
 if (conn->uri->user) {
@@ -692,7 +692,7 @@ esxConnectToVCenter(esxPrivate *priv,
 const char *hostSystemIPAddress)
 {
 int result = -1;
-char ipAddress[NI_MAXHOST] = "";
+g_autofree char *ipAddress = NULL;
 char *username = NULL;
 char *password = NULL;
 char *url = NULL;
@@ -704,7 +704,7 @@ esxConnectToVCenter(esxPrivate *priv,
 return -1;
 }
 
-if (esxUtil_ResolveHostname(hostname, ipAddress, NI_MAXHOST) < 0)
+if (esxUtil_ResolveHostname(hostname, ) < 0)
 return -1;
 
 if (conn->uri->user) {
@@ -813,7 +813,7 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
 virDrvOpenStatus result = VIR_DRV_OPEN_ERROR;
 esxPrivate *priv = NULL;
 char *potentialVCenterIPAddress = NULL;
-char vCenterIPAddress[NI_MAXHOST] = "";
+g_autofree char *vCenterIPAddress = NULL;
 
 virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
 
@@ -875,16 +875,10 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
 goto cleanup;
 }
 
-if (virStrcpyStatic(vCenterIPAddress,
-potentialVCenterIPAddress) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("vCenter IP address %s too big for 
destination"),
-   potentialVCenterIPAddress);
-goto cleanup;
-}
+vCenterIPAddress = g_strdup(potentialVCenterIPAddress);
 } else {
 if (esxUtil_ResolveHostname(priv->parsedUri->vCenter,
-vCenterIPAddress, NI_MAXHOST) < 0) 
{
+) < 0) {
 goto cleanup;
 }
 
diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index 555158f953..d9e7641d67 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -278,12 +278,12 @@ esxUtil_ParseDatastorePath(const char *datastorePath, 
char **datastoreName,
 
 
 int
-esxUtil_ResolveHostname(const char *hostname,
-char *ipAddress, size_t ipAddress_length)
+esxUtil_ResolveHostname(const char *hostname, char **ipAddress)
 {
 struct addrinfo hints;
 struct addrinfo *result = NULL;
 int errcode;
+g_autofree char *address = NULL;
 
 memset(, 0, sizeof(hints));
 
@@ -308,8 +308,9 @@ esxUtil_ResolveHostname(const char *hostname,
 return -1;
 }
 
-errcode = getnameinfo(result->ai_addr, result->ai_addrlen, ipAddress,
-  ipAddress_length, NULL, 0, NI_NUMERICHOST);
+address = g_new0(char, NI_MAXHOST);
+errcode = getnameinfo(result->ai_addr, result->ai_addrlen, address,
+  NI_MAXHOST, NULL, 0, NI_NUMERICHOST);
 freeaddrinfo(result);
 
 if (errcode != 0) {
@@ -319,6 +320,8 @@ esxUtil_ResolveHostname(const char *hostname,
 return -1;
 }
 
+*ipAddress = g_strdup(address);
+
 return 0;
 }
 
diff --git a/src/esx/esx_util.h b/src/esx/esx_util.h
index 97b6d82a2b..9bfbff1d42 100644
--- a/src/esx/esx_util.h
+++ b/src/esx/esx_util.h
@@ -55,8 +55,7 @@ int esxUtil_ParseVirtualMachineIDString(const char 
*id_string, int *id);
 int esxUtil_ParseDatastorePath(const char *datastorePath, char **datastoreName,
char **directoryName, char 
**directoryAndFileName);
 
-int esxUtil_ResolveHostname(const char *hostname,
-char *ipAddress, size_t ipAddress_length);
+int esxUtil_ResolveHostname(const char *hostname, char **ipAddress);
 
 int esxUtil_ReformatUuid(const char *input, char *output);
 
-- 
2.26.2



[libvirt PATCH 1/2] esx: call freeaddrinfo earlier in esxUtil_ResolveHostname

2020-10-05 Thread Pino Toscano
Call freeaddrinfo() as soon as @result is not needed anymore, i.e. right
after getnameinfo(); this avoids calling freeaddrinfo() in two branches.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_util.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index 9100873326..555158f953 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -310,17 +310,15 @@ esxUtil_ResolveHostname(const char *hostname,
 
 errcode = getnameinfo(result->ai_addr, result->ai_addrlen, ipAddress,
   ipAddress_length, NULL, 0, NI_NUMERICHOST);
+freeaddrinfo(result);
 
 if (errcode != 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Formatting IP address for host '%s' failed: %s"), 
hostname,
gai_strerror(errcode));
-freeaddrinfo(result);
 return -1;
 }
 
-freeaddrinfo(result);
-
 return 0;
 }
 
-- 
2.26.2



Re: [libvirt PATCH 5/8] esx: esxConnectOpen: use allocated buffer

2020-10-05 Thread Pino Toscano
On Monday, 5 October 2020 00:21:42 CEST Ján Tomko wrote:
> Signed-off-by: Ján Tomko 
> ---
>  src/esx/esx_driver.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> index e82e5ed835..0798493296 100644
> --- a/src/esx/esx_driver.c
> +++ b/src/esx/esx_driver.c
> @@ -813,7 +813,7 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
>  virDrvOpenStatus result = VIR_DRV_OPEN_ERROR;
>  esxPrivate *priv = NULL;
>  char *potentialVCenterIPAddress = NULL;
> -char vCenterIPAddress[NI_MAXHOST] = "";
> +g_autofree char *vCenterIPAddress = g_new0(char, NI_MAXHOST);

Hmm, this is not the only char[NI_MAXHOST] in this file, so I'm
surprised only this one triggered the stack limit check.

The NI_MAXHOST-limited buffer seems to be due to
esxUtil_ResolveHostname(), which forces it due to its interface.
I'll rewrite it to return a new string instead; consider it as a NACK
for this patch.

-- 
Pino Toscano

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


Re: [libvirt PATCH] API: virDomainLookupByID: s/UUId/UUID/

2020-10-04 Thread Pino Toscano
On Monday, 5 October 2020 00:25:53 CEST Ján Tomko wrote:
> Signed-off-by: Ján Tomko 
> ---
>  src/libvirt-domain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 415482a526..a7266ccd88 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -284,7 +284,7 @@ virDomainCreateLinux(virConnectPtr conn, const char 
> *xmlDesc,
>   *
>   * Try to find a domain based on the hypervisor ID number
>   * Note that this won't work for inactive domains which have an ID of -1,
> - * in that case a lookup based on the Name or UUId need to be done instead.
> + * in that case a lookup based on the Name or UUID need to be done instead.
>   *
>   * virDomainFree should be used to free the resources after the
>   * domain object is no longer needed.

Reviewed-by: Pino Toscano 

-- 
Pino Toscano

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


Re: [PATCH 7/8] hyperv: implement connectGetVersion

2020-10-04 Thread Pino Toscano
On Saturday, 3 October 2020 01:02:47 CEST Matt Coleman wrote:
> On Oct 2, 2020, at 2:48 AM, Pino Toscano  wrote:
> > IMHO these explanations should be documented in the Hyper-V driver
> > page: docs/drvhyperv.html.in. This way, users know about the different
> > value returned by virConnectGetVersion() by the Hyper-V driver, and
> > can unmangle it to get the real Hyper-V version.
> 
> You’re absolutely right. I’ll add some documentation about this. Would 
> you prefer it as a separate commit or included with the code changes?

IMHO it makes sense in the same commit, as it would be the new logic
and the documentation for it (in both code comments and rST for users)
added at the same time.

-- 
Pino Toscano

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


Re: [PATCH 3/8] hyperv: break out common lookups into separate functions

2020-10-04 Thread Pino Toscano
On Friday, 2 October 2020 23:50:22 CEST Matt Coleman wrote:
> On Oct 2, 2020, at 2:25 AM, Pino Toscano  wrote:
> > Note that now hypervGetVirtualSystemByID() issues VIR_ERR_INTERNAL_ERROR
> > in case  / *computerSystemList is null, instead of
> > VIR_ERR_NO_DOMAIN. Shouldn't this still be able to explicitly report
> > when the requested domain does not exist?
> 
> I’ll split it up into two conditionals. Would you prefer to see the NULL 
> check that produces the VIR_ERR_NO_DOMAIN error in the new helper 
> function (hypervGetVirtualSystemByID) or its caller 
> (hypervDomainLookupByID)?

This depends on the callers: if the logic for checking whether a domain
is considered "not already existing" makes sense for all the callers of
hypervGetVirtualSystemByID, then I'd say to add it there.

-- 
Pino Toscano

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


Re: [PATCH 7/8] hyperv: implement connectGetVersion

2020-10-02 Thread Pino Toscano
On Thursday, 1 October 2020 23:47:16 CEST mcole...@datto.com wrote:
> From: Matt Coleman 
> 
> Hyper-V version numbers are not compatible with the encoding in
> virParseVersionString():
> https://gitlab.com/libvirt/libvirt/-/blob/master/src/util/virutil.c#L246
> 
> For example, the Windows Server 2016 Hyper-V version is 10.0.14393: its
> micro is over 14 times larger than the encoding allows.
> 
> This commit repacks the Hyper-V version number in order to preserve all
> of the digits. The major and minor are concatenated (with minor zero-
> padded to two digits) to form the repacked major value. This works
> because Microsoft's major and minor versions numbers are unlikely to
> exceed 99. The repacked minor value is derived from the digits in the
> thousands, ten-thousands, and hundred-thousands places of Hyper-V's
> micro. The repacked micro is derived from the digits in the ones, tens,
> and hundreds places of Hyper-V's micro.
> [...]
> +/*
> + * Mangle the version to work with virParseVersionString() while 
> retaining all the digits.
> + *
> + * For example...
> + * 2008:  6.0.6001 =>   600.6.1
> + * 2008 R2:   6.1.7600 =>   601.7.600
> + * 2012:  6.2.9200 =>   602.9.200
> + * 2012 R2:   6.3.9600 =>   603.9.600
> + * 2016:  10.0.14393   =>   1000.14.393
> + * 2019:  10.0.17763   =>   1000.17.763
> + */

IMHO these explanations should be documented in the Hyper-V driver
page: docs/drvhyperv.html.in. This way, users know about the different
value returned by virConnectGetVersion() by the Hyper-V driver, and
can unmangle it to get the real Hyper-V version.

> +if (virStrToLong_ui(os->data.common->Version, , 10, ) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +  _("Could not parse major from '%s'"),
> +  os->data.common->Version);
> +goto cleanup;
> +}
> +
> +if (virStrToLong_ui(suffix + 1, , 10, ) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +  _("Could not parse minor from '%s'"),
> +  os->data.common->Version);
> +goto cleanup;
> +}
> +
> +if (virStrToLong_ui(suffix + 1, NULL, 10, ) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +  _("Could not parse micro from '%s'"),
> +  os->data.common->Version);
> +goto cleanup;
> +}
> +
> +if (major > 99 || minor > 99 || micro > 99) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Could not produce mangled version number from 
> '%s'"),
> +   os->data.common->Version);
> +goto cleanup;
> +}

I'd move the parsing code to an own helper, similar to
virParseVersionString():

  int
  hypervParseVersionString(const char *str, unsigned int *major,
   unsigned int *minor, unsigned int *micro)

without virReportError() in it; then use a single virReportError() for
hypervParseVersionString() failure.

> +
> +virBufferAsprintf(_version, "%d%02d.%d.%d", major, minor,
> +  micro > 999 ? micro / 1000 : 0,
> +  micro > 999 ? micro % 1000 : micro);
> +
> +/* Parse version string to long */
> +if (virParseVersionString(virBufferCurrentContent(_version), 
> version, true) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Could not parse version number from '%s'"),
> +   os->data.common->Version);
> +goto cleanup;
> +}

Considering the major, minor and micro parts of the version string are
already broken out here, why not directly encode the version number as
long? It seems a forced roundtrip to format it as string, and parse it
again.

> diff --git a/src/hyperv/hyperv_wmi_generator.input 
> b/src/hyperv/hyperv_wmi_generator.input
> index 77543cf6ef..bbca550790 100644
> --- a/src/hyperv/hyperv_wmi_generator.input
> +++ b/src/hyperv/hyperv_wmi_generator.input
> @@ -673,7 +673,7 @@ class Win32_OperatingSystem
>  string   CSCreationClassName
>  string   CSDVersion
>  string   CSName
> -uint16   CurrentTimeZone
> +int16CurrentTimeZone
>  boolean  DataExecutionPrevention_Available
>  boolean  DataExecutionPrevention_32BitApplications
>  boolean  DataExecutionPrevention_Drivers

This seems unrelated to the patch? Or it is needed because this class
is used by the hypervGetWmiClass(Win32_OperatingSystem, ...) call, and
the field needed to be fixed?

I'd split it as separate commit before this one.

-- 
Pino Toscano

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


Re: [PATCH 4/8] hyperv: replace generic WMI class list helpers with a macro

2020-10-02 Thread Pino Toscano
On Thursday, 1 October 2020 23:47:13 CEST mcole...@datto.com wrote:
> diff --git a/src/hyperv/hyperv_wmi.h b/src/hyperv/hyperv_wmi.h
> index 74a74e0e09..c493a45691 100644
> --- a/src/hyperv/hyperv_wmi.h
> +++ b/src/hyperv/hyperv_wmi.h
> @@ -37,9 +37,6 @@
>  #define MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_SELECTOR \
>  "CreationClassName=Msvm_VirtualSystemManagementService"
>  
> -int hypervVerifyResponse(WsManClient *client, WsXmlDocH response,
> - const char *detail);
> -

This change seems unrelated to the patch. Can you please split it in an
own commit?

-- 
Pino Toscano

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


Re: [PATCH 3/8] hyperv: break out common lookups into separate functions

2020-10-02 Thread Pino Toscano
> flags)
>  if (hypervMsvmComputerSystemFromDomain(domain, ) < 0)
>  goto cleanup;
>  
> -/* Get Msvm_VirtualSystemSettingData */
> -virBufferEscapeSQL(,
> -   "associators of "
> -   
> "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\","
> -   "Name=\"%s\"} "
> -   "where AssocClass = Msvm_SettingsDefineState "
> -   "ResultClass = Msvm_VirtualSystemSettingData",
> -   uuid_string);
> -
> -if (hypervGetMsvmVirtualSystemSettingDataList(priv, ,
> -  ) 
> < 0) {
> -goto cleanup;
> -}
> -
> -if (virtualSystemSettingData == NULL) {
> +if (hypervGetVSSDFromUUID(priv, uuid_string,
> +  ) < 0) {
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Could not lookup %s for domain %s"),
> "Msvm_VirtualSystemSettingData",

Ditto.

> @@ -788,20 +859,9 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int 
> flags)
>  goto cleanup;
>  }
>  
> -/* Get Msvm_ProcessorSettingData */
> -virBufferEscapeSQL(,
> -   "associators of "
> -   "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} "
> -   "where AssocClass = 
> Msvm_VirtualSystemSettingDataComponent "
> -   "ResultClass = Msvm_ProcessorSettingData",
> -   virtualSystemSettingData->data.common->InstanceID);
> -
> -if (hypervGetMsvmProcessorSettingDataList(priv, ,
> -  ) < 0) {
> -goto cleanup;
> -}
> -
> -if (processorSettingData == NULL) {
> +if (hypervGetProcSDByVSSDInstanceId(priv,
> +       virtualSystemSettingData->data.common->InstanceID,
> +   ) < 0) {
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Could not lookup %s for domain %s"),
> "Msvm_ProcessorSettingData",

Ditto.

> @@ -809,21 +869,9 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int 
> flags)
>  goto cleanup;
>  }
>  
> -/* Get Msvm_MemorySettingData */
> -virBufferEscapeSQL(,
> -   "associators of "
> -   "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} "
> -   "where AssocClass = 
> Msvm_VirtualSystemSettingDataComponent "
> -   "ResultClass = Msvm_MemorySettingData",
> -   virtualSystemSettingData->data.common->InstanceID);
> -
> -if (hypervGetMsvmMemorySettingDataList(priv, ,
> -   ) < 0) {
> -goto cleanup;
> -}
> -
> -
> -if (memorySettingData == NULL) {
> +if (hypervGetMemSDByVSSDInstanceId(priv,
> +   virtualSystemSettingData->data.common->InstanceID,
> +   ) < 0) {
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Could not lookup %s for domain %s"),
> "Msvm_MemorySettingData",

Ditto.

-- 
Pino Toscano

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


Re: [PATCH 0/8] hyperv: implement new APIs & more

2020-10-02 Thread Pino Toscano
On Thursday, 1 October 2020 23:47:09 CEST mcole...@datto.com wrote:
> From: Matt Coleman 
> 
> These patches fix a couple bugs, consolidate duplicate code, and 
> implement several APIs.

Nice!

One general note for all the patches: NEWS.rst is updated as a whole
at the end, as separate commit. This eases cherry-picking, removes
dependencies between patches that would not conflict otherwise, and
avoids churn in each patch.

Thanks,
-- 
Pino Toscano

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


[libvirt PATCH] build: remove old macvtap and virtualport leftovers

2020-10-01 Thread Pino Toscano
Followup of commit a79e7639daffac04b088378e6c79854fcac292f3 and
commit 7556ab139fd2e503ac26ee232ab273f1ec027c21.

Signed-off-by: Pino Toscano 
---
 meson.build | 10 --
 1 file changed, 10 deletions(-)

diff --git a/meson.build b/meson.build
index 257e4452a1..a5ce8e17a8 100644
--- a/meson.build
+++ b/meson.build
@@ -1503,10 +1503,6 @@ elif get_option('firewalld_zone').enabled()
   error('You must have firewalld support enabled to enable firewalld_zone')
 endif
 
-if conf.has('WITH_MACVTAP') and not conf.has('WITH_LIBNL')
-  error('libnl3-devel is required for macvtap support')
-endif
-
 if (pkcheck_prog.found() or get_option('polkit').enabled())
   conf.set('WITH_POLKIT', 1)
 endif
@@ -1515,10 +1511,6 @@ if udev_dep.found() and not pciaccess_dep.found()
   error('You must install the pciaccess module to build with udev')
 endif
 
-if conf.has('WITH_VIRTUALPORT') and not conf.has('WITH_MACVTAP')
-  error('macvtap is required for virtualport support')
-endif
-
 
 # build driver options
 
@@ -2388,7 +2380,6 @@ libs_summary = {
   'libssh': libssh_dep.found(),
   'libssh2': libssh2_dep.found(),
   'libxml': libxml_dep.found(),
-  'macvtap': conf.has('WITH_MACVTAP'),
   'netcf': netcf_dep.found(),
   'NLS': have_gnu_gettext_tools,
   'nss': conf.has('WITH_NSS'),
@@ -2403,7 +2394,6 @@ libs_summary = {
   'sasl': sasl_dep.found(),
   'selinux': selinux_dep.found(),
   'udev': udev_dep.found(),
-  'virtualport': conf.has('WITH_VIRTUALPORT'),
   'xdr': xdr_dep.found(),
   'yajl': yajl_dep.found(),
 }
-- 
2.26.2



[libvirt PATCH] gitdm: add more individual contributors

2020-09-25 Thread Pino Toscano
Signed-off-by: Pino Toscano 
---
 docs/gitdm/groups/unaffiliated | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/docs/gitdm/groups/unaffiliated b/docs/gitdm/groups/unaffiliated
index 5f867fc5e4..1ca7eee81f 100644
--- a/docs/gitdm/groups/unaffiliated
+++ b/docs/gitdm/groups/unaffiliated
@@ -14,6 +14,7 @@ gmx.de
 googlemail.com
 hotmail.com
 mail.ru
+mailbox.org
 outlook.com
 pobox.com
 protonmail.com
@@ -43,11 +44,14 @@ c...@cky.nz
 cont...@puzio.waw.pl
 d.herrendoer...@herrendoerfer.name
 d...@danny.cz
+da...@davemloft.net
 de...@fobos.de
 e...@sf-mail.de
 e...@tty.sk
+fed...@borsoi.fr
 fr...@fritz-elfert.de
 g...@czarc.net
+g...@ryandesign.com
 gor...@dragonsdawn.net
 gre...@kopka.net
 heathpeter...@kandre.com
@@ -59,9 +63,11 @@ j...@ristioja.ee
 james...@cowgill.org.uk
 ja...@shubin.ca
 jas...@humppa.nl
+jean-bapti...@holcroft.fr
 jer...@goop.org
 j...@ozlabs.org
 j...@horde.net
+ke...@kevinlocke.name
 kl...@ethgen.de
 la...@caesar.elte.hu
 len...@lhuard.fr.eu.org
@@ -71,6 +77,7 @@ ma...@juffo.org
 m...@rfc2324.org
 mich...@ellerman.id.au
 m...@very.puzzling.org
+m...@milo.name
 n0...@n0ano.com
 n...@aldur.co.uk
 nob...@nowhere.ws
@@ -81,11 +88,14 @@ rich...@nod.at
 r...@tigress.co.uk
 ru...@rubenkerkhof.com
 r...@rufoa.com
+scott-libv...@shambarger.net
+shemmin...@vyatta.com
 sla...@kaplonski.pl
 soulxu@soulxu-thinkpad-t410.(none)
 sty...@turnovfree.net
 t...@rakugaki.org
 tho...@scripty.at
+toscano.p...@tiscali.it
 v.tols...@selfip.ru
 ville.sky...@iki.fi
 vinc...@bernat.im
-- 
2.26.2



[libvirt PATCH] news: document recent esx API implementations

2020-09-24 Thread Pino Toscano
Signed-off-by: Pino Toscano 
---
 NEWS.rst | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/NEWS.rst b/NEWS.rst
index e992fbe471..7c723e6610 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -32,6 +32,13 @@ v6.8.0 (unreleased)
 can now be passed using the ``passwd`` attribute on
 the  element.
 
+  * esx: implement few APIs
+
+The ``virConnectListAllNetworks()``, ``virDomainGetHostname()``, and
+``virDomainInterfaceAddresses()`` (only for
+``VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT`` source) APIs were implemented
+in the esx driver.
+
 * **Improvements**
 
   * qemu: Allow migration over UNIX sockets
-- 
2.26.2



[libvirt PATCH v2 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 | 170 +++
 2 files changed, 174 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..5a742ed3df 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -5113,6 +5113,175 @@ 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) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Missing property '%s' in answer"),
+   "guest.net");
+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;

[libvirt PATCH v2 1/2] esx: generator: add GuestNicInfo object

2020-09-14 Thread Pino Toscano
Add the definition of the GuestNicInfo object, with all the required
objects for it.

Signed-off-by: Pino Toscano 
---
 scripts/esx_vi_generator.py|  1 +
 src/esx/esx_vi_generator.input | 54 ++
 2 files changed, 55 insertions(+)

diff --git a/scripts/esx_vi_generator.py b/scripts/esx_vi_generator.py
index e0782e35f3..7929e1e682 100755
--- a/scripts/esx_vi_generator.py
+++ b/scripts/esx_vi_generator.py
@@ -1292,6 +1292,7 @@ additional_object_features = {
 "DatastoreHostMount": (Object.FEATURE__DEEP_COPY | Object.FEATURE__LIST |
Object.FEATURE__ANY_TYPE),
 "DatastoreInfo": Object.FEATURE__ANY_TYPE | Object.FEATURE__DYNAMIC_CAST,
+"GuestNicInfo": Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE,
 "HostConfigManager": Object.FEATURE__ANY_TYPE,
 "HostCpuIdInfo": Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE,
 "HostDatastoreBrowserSearchResults": (Object.FEATURE__LIST |
diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input
index 22c114e0aa..bd6ac72a18 100644
--- a/src/esx/esx_vi_generator.input
+++ b/src/esx/esx_vi_generator.input
@@ -277,6 +277,18 @@ object FolderFileQuery   extends FileQuery
 end
 
 
+object GuestNicInfo
+Boolean  connected  r
+Int  deviceConfigId r
+NetDnsConfigInfo dnsConfig  o
+String   ipAddress  ol
+NetIpConfigInfo  ipConfig   o
+String   macAddress o
+NetBIOSConfigInfonetBIOSConfig  o
+String   networko
+end
+
+
 object HostAutoStartManagerConfig
 AutoStartDefaultsdefaults   o
 AutoStartPowerInfo   powerInfo  ol
@@ -770,6 +782,48 @@ object NasDatastoreInfo  extends DatastoreInfo
 end
 
 
+object NetBIOSConfigInfo
+String   mode   r
+end
+
+
+object NetDhcpConfigInfo
+NetDhcpConfigInfoDhcpOptions ipv4   o
+NetDhcpConfigInfoDhcpOptions ipv6   o
+end
+
+
+object NetDhcpConfigInfoDhcpOptions
+KeyAnyValue  config ol
+Boolean  enable r
+end
+
+
+object NetDnsConfigInfo
+Boolean  dhcp   r
+String   domainName r
+String   hostName   r
+String   ipAddress  ol
+String   searchDomain   ol
+end
+
+
+object NetIpConfigInfo
+Boolean  autoConfigurationEnabled   o
+NetDhcpConfigInfodhcp   o
+NetIpConfigInfoIpAddress ipAddress  ol
+end
+
+
+object NetIpConfigInfoIpAddress
+String   ipAddress  r
+DateTime lifetime   o
+String   origin o
+Int  prefixLength   r
+String   state  o
+end
+
+
 object ObjectContent
 ManagedObjectReference   objr
 DynamicProperty  propSetol
-- 
2.26.2



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.


[libvirt PATCH v2] esx: implement connectListAllNetworks

2020-09-14 Thread Pino Toscano
Implement the .connectListAllNetworks networks API in the esx network
driver.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_network_driver.c | 69 
 1 file changed, 69 insertions(+)

diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index 88843aae2f..a4e738ba72 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -863,6 +863,74 @@ esxNetworkIsPersistent(virNetworkPtr network G_GNUC_UNUSED)
 
 
 
+#define MATCH(FLAG) (flags & (FLAG))
+static int
+esxConnectListAllNetworks(virConnectPtr conn,
+  virNetworkPtr **nets,
+  unsigned int flags)
+{
+int ret = -1;
+esxPrivate *priv = conn->privateData;
+esxVI_HostVirtualSwitch *hostVirtualSwitchList = NULL;
+esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL;
+size_t count = 0;
+size_t i;
+
+virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
+
+/*
+ * ESX networks are always active, persistent, and
+ * autostarted, so return zero elements in case we are asked
+ * for networks different than that.
+ */
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_ACTIVE) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_ACTIVE)))
+return 0;
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_PERSISTENT) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_PERSISTENT)))
+return 0;
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_AUTOSTART) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_AUTOSTART)))
+return 0;
+
+if (esxVI_EnsureSession(priv->primary) < 0 ||
+esxVI_LookupHostVirtualSwitchList(priv->primary,
+  ) < 0) {
+return -1;
+}
+
+for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch;
+ hostVirtualSwitch = hostVirtualSwitch->_next) {
+if (nets) {
+virNetworkPtr net = virtualswitchToNetwork(conn, 
hostVirtualSwitch);
+if (!net)
+goto cleanup;
+if (VIR_APPEND_ELEMENT(*nets, count, net) < 0)
+goto cleanup;
+} else {
+++count;
+}
+}
+
+ret = count;
+
+ cleanup:
+if (ret < 0) {
+if (nets && *nets) {
+for (i = 0; i < count; ++i)
+VIR_FREE((*nets)[i]);
+VIR_FREE(*nets);
+}
+}
+
+esxVI_HostVirtualSwitch_Free();
+
+return ret;
+}
+#undef MATCH
+
+
+
 virNetworkDriver esxNetworkDriver = {
 .connectNumOfNetworks = esxConnectNumOfNetworks, /* 0.10.0 */
 .connectListNetworks = esxConnectListNetworks, /* 0.10.0 */
@@ -877,4 +945,5 @@ virNetworkDriver esxNetworkDriver = {
 .networkSetAutostart = esxNetworkSetAutostart, /* 0.10.0 */
 .networkIsActive = esxNetworkIsActive, /* 0.10.0 */
 .networkIsPersistent = esxNetworkIsPersistent, /* 0.10.0 */
+.connectListAllNetworks = esxConnectListAllNetworks, /* 6.8.0 */
 };
-- 
2.26.2



Re: [libvirt PATCH] esx: implement connectListAllNetworks

2020-09-14 Thread Pino Toscano
On Monday, 14 September 2020 17:16:50 CEST Michal Privoznik wrote:
> On 9/14/20 11:10 AM, Pino Toscano wrote:
> > Implement the .connectListAllNetworks networks API in the esx network
> > driver.
> > 
> > Signed-off-by: Pino Toscano 
> > ---
> >   src/esx/esx_network_driver.c | 64 
> >   1 file changed, 64 insertions(+)
> > 
> > diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
> > index 88843aae2f..5d9c1e3c2c 100644
> > --- a/src/esx/esx_network_driver.c
> > +++ b/src/esx/esx_network_driver.c
> > @@ -863,6 +863,69 @@ esxNetworkIsPersistent(virNetworkPtr network 
> > G_GNUC_UNUSED)
> >   
> >   
> >   
> > +#define MATCH(FLAG) (flags & (FLAG))
> > +static int
> > +esxConnectListAllNetworks(virConnectPtr conn,
> > +  virNetworkPtr **nets,
> > +  unsigned int flags)
> > +{
> > +int ret = -1;
> > +esxPrivate *priv = conn->privateData;
> > +esxVI_HostVirtualSwitch *hostVirtualSwitchList = NULL;
> > +esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL;
> > +size_t count = 0;
> > +size_t i;
> > +
> > +virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
> > +
> > +/*
> > + * ESX networks are always active, persistent, and
> > + * autostarted, so return zero elements in case we are asked
> > + * for networks different than that.
> > + */
> > +if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_ACTIVE) &&
> > +!(MATCH(VIR_CONNECT_LIST_NETWORKS_ACTIVE)))
> > +return 0;
> > +if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_PERSISTENT) &&
> > +!(MATCH(VIR_CONNECT_LIST_NETWORKS_PERSISTENT)))
> > +return 0;
> > +if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_AUTOSTART) &&
> > +!(MATCH(VIR_CONNECT_LIST_NETWORKS_AUTOSTART)))
> > +return 0;
> > +
> > +if (esxVI_EnsureSession(priv->primary) < 0 ||
> > +esxVI_LookupHostVirtualSwitchList(priv->primary,
> > +  ) < 0) {
> > +return -1;
> > +}
> > +
> > +for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch;
> > + hostVirtualSwitch = hostVirtualSwitch->_next) {
> > +virNetworkPtr net = virtualswitchToNetwork(conn, 
> > hostVirtualSwitch);
> > +
> > +if (VIR_APPEND_ELEMENT(*nets, count, net) < 0)
> 
> The virConnectListAllNetworks() documents that @nets can be NULL if the 
> caller is interested only in the count of networks.

Ah, I missed that bit, thanks. I will send v2 with a tweaked version.

Thanks,
-- 
Pino Toscano

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


[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,
+

[libvirt PATCH 1/2] esx: generator: add GuestNicInfo object

2020-09-14 Thread Pino Toscano
Add the definition of the GuestNicInfo object, with all the required
objects for it.

Signed-off-by: Pino Toscano 
---
 scripts/esx_vi_generator.py|  1 +
 src/esx/esx_vi_generator.input | 54 ++
 2 files changed, 55 insertions(+)

diff --git a/scripts/esx_vi_generator.py b/scripts/esx_vi_generator.py
index e0782e35f3..7929e1e682 100755
--- a/scripts/esx_vi_generator.py
+++ b/scripts/esx_vi_generator.py
@@ -1292,6 +1292,7 @@ additional_object_features = {
 "DatastoreHostMount": (Object.FEATURE__DEEP_COPY | Object.FEATURE__LIST |
Object.FEATURE__ANY_TYPE),
 "DatastoreInfo": Object.FEATURE__ANY_TYPE | Object.FEATURE__DYNAMIC_CAST,
+"GuestNicInfo": Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE,
 "HostConfigManager": Object.FEATURE__ANY_TYPE,
 "HostCpuIdInfo": Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE,
 "HostDatastoreBrowserSearchResults": (Object.FEATURE__LIST |
diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input
index 22c114e0aa..bd6ac72a18 100644
--- a/src/esx/esx_vi_generator.input
+++ b/src/esx/esx_vi_generator.input
@@ -277,6 +277,18 @@ object FolderFileQuery   extends FileQuery
 end
 
 
+object GuestNicInfo
+Boolean  connected  r
+Int  deviceConfigId r
+NetDnsConfigInfo dnsConfig  o
+String   ipAddress  ol
+NetIpConfigInfo  ipConfig   o
+String   macAddress o
+NetBIOSConfigInfonetBIOSConfig  o
+String   networko
+end
+
+
 object HostAutoStartManagerConfig
 AutoStartDefaultsdefaults   o
 AutoStartPowerInfo   powerInfo  ol
@@ -770,6 +782,48 @@ object NasDatastoreInfo  extends DatastoreInfo
 end
 
 
+object NetBIOSConfigInfo
+String   mode   r
+end
+
+
+object NetDhcpConfigInfo
+NetDhcpConfigInfoDhcpOptions ipv4   o
+NetDhcpConfigInfoDhcpOptions ipv6   o
+end
+
+
+object NetDhcpConfigInfoDhcpOptions
+KeyAnyValue  config ol
+Boolean  enable r
+end
+
+
+object NetDnsConfigInfo
+Boolean  dhcp   r
+String   domainName r
+String   hostName   r
+String   ipAddress  ol
+String   searchDomain   ol
+end
+
+
+object NetIpConfigInfo
+Boolean  autoConfigurationEnabled   o
+NetDhcpConfigInfodhcp   o
+NetIpConfigInfoIpAddress ipAddress  ol
+end
+
+
+object NetIpConfigInfoIpAddress
+String   ipAddress  r
+DateTime lifetime   o
+String   origin o
+Int  prefixLength   r
+String   state  o
+end
+
+
 object ObjectContent
 ManagedObjectReference   objr
 DynamicProperty  propSetol
-- 
2.26.2



[libvirt PATCH] esx: implement connectListAllNetworks

2020-09-14 Thread Pino Toscano
Implement the .connectListAllNetworks networks API in the esx network
driver.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_network_driver.c | 64 
 1 file changed, 64 insertions(+)

diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index 88843aae2f..5d9c1e3c2c 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -863,6 +863,69 @@ esxNetworkIsPersistent(virNetworkPtr network G_GNUC_UNUSED)
 
 
 
+#define MATCH(FLAG) (flags & (FLAG))
+static int
+esxConnectListAllNetworks(virConnectPtr conn,
+  virNetworkPtr **nets,
+  unsigned int flags)
+{
+int ret = -1;
+esxPrivate *priv = conn->privateData;
+esxVI_HostVirtualSwitch *hostVirtualSwitchList = NULL;
+esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL;
+size_t count = 0;
+size_t i;
+
+virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
+
+/*
+ * ESX networks are always active, persistent, and
+ * autostarted, so return zero elements in case we are asked
+ * for networks different than that.
+ */
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_ACTIVE) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_ACTIVE)))
+return 0;
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_PERSISTENT) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_PERSISTENT)))
+return 0;
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_AUTOSTART) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_AUTOSTART)))
+return 0;
+
+if (esxVI_EnsureSession(priv->primary) < 0 ||
+esxVI_LookupHostVirtualSwitchList(priv->primary,
+  ) < 0) {
+return -1;
+}
+
+for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch;
+ hostVirtualSwitch = hostVirtualSwitch->_next) {
+virNetworkPtr net = virtualswitchToNetwork(conn, hostVirtualSwitch);
+
+if (VIR_APPEND_ELEMENT(*nets, count, net) < 0)
+goto cleanup;
+}
+
+ret = count;
+
+ cleanup:
+if (ret < 0) {
+if (*nets) {
+for (i = 0; i < count; ++i)
+VIR_FREE((*nets)[i]);
+VIR_FREE(*nets);
+}
+}
+
+esxVI_HostVirtualSwitch_Free();
+
+return ret;
+}
+#undef MATCH
+
+
+
 virNetworkDriver esxNetworkDriver = {
 .connectNumOfNetworks = esxConnectNumOfNetworks, /* 0.10.0 */
 .connectListNetworks = esxConnectListNetworks, /* 0.10.0 */
@@ -877,4 +940,5 @@ virNetworkDriver esxNetworkDriver = {
 .networkSetAutostart = esxNetworkSetAutostart, /* 0.10.0 */
 .networkIsActive = esxNetworkIsActive, /* 0.10.0 */
 .networkIsPersistent = esxNetworkIsPersistent, /* 0.10.0 */
+.connectListAllNetworks = esxConnectListAllNetworks, /* 6.8.0 */
 };
-- 
2.26.2



[libvirt PATCH] esx: implement domainGetHostname

2020-09-10 Thread Pino Toscano
Implement the .domainGetHostname hypervisor driver API to get the
hostname of a running guest (needs VMware Tools).

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

diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in
index e25cf07e92..38de640c2a 100644
--- a/docs/drvesx.html.in
+++ b/docs/drvesx.html.in
@@ -796,6 +796,9 @@ Enter administrator password for example-vcenter.com:
 performed the ESX server raises an error and the driver reports it.
 
 
+
+virDomainGetHostname
+
 
 virDomainReboot
 
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 9080478435..bddc588977 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -5060,6 +5060,59 @@ esxDomainHasManagedSaveImage(virDomainPtr domain, 
unsigned int flags)
 }
 
 
+static char *
+esxDomainGetHostname(virDomainPtr domain,
+ unsigned int flags)
+{
+esxPrivate *priv = domain->conn->privateData;
+esxVI_String *propertyNameList = NULL;
+esxVI_ObjectContent *virtualMachine = NULL;
+esxVI_VirtualMachinePowerState powerState;
+char *hostname = NULL;
+char *new_hostname = NULL;
+
+virCheckFlags(0, NULL);
+
+if (esxVI_EnsureSession(priv->primary) < 0)
+return NULL;
+
+if (esxVI_String_AppendValueListToList(,
+   "runtime.powerState\0"
+   "guest.hostName") < 0 ||
+esxVI_LookupVirtualMachineByUuid(priv->primary, domain->uuid,
+ propertyNameList, ,
+ esxVI_Occurrence_OptionalItem) ||
+esxVI_GetVirtualMachinePowerState(virtualMachine, ) < 0) {
+goto cleanup;
+}
+
+if (powerState != esxVI_VirtualMachinePowerState_PoweredOn) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("Domain is not powered on"));
+goto cleanup;
+}
+
+if (esxVI_GetStringValue(virtualMachine, "guest.hostName",
+ , esxVI_Occurrence_OptionalItem) < 0) {
+goto cleanup;
+}
+
+if (!hostname) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("hostName field not available (missing VMware 
Tools?)"));
+goto cleanup;
+}
+
+new_hostname = g_strdup(hostname);
+
+ cleanup:
+esxVI_String_Free();
+esxVI_ObjectContent_Free();
+
+return new_hostname;
+}
+
+
 static virHypervisorDriver esxHypervisorDriver = {
 .name = "ESX",
 .connectOpen = esxConnectOpen, /* 0.7.0 */
@@ -5140,6 +5193,7 @@ static virHypervisorDriver esxHypervisorDriver = {
 .domainSnapshotDelete = esxDomainSnapshotDelete, /* 0.8.0 */
 .connectIsAlive = esxConnectIsAlive, /* 0.9.8 */
 .domainHasManagedSaveImage = esxDomainHasManagedSaveImage, /* 1.2.13 */
+.domainGetHostname = esxDomainGetHostname, /* 6.8.0 */
 };
 
 
-- 
2.26.2



[libvirt PATCH v2] esx: improve some of the virErrorNumber used

2020-09-10 Thread Pino Toscano
A lot of virReportError() calls use VIR_ERR_INTERNAL_ERROR to represent
the number of the error, even in cases where there is one fitting more.
Hence, replace some of them with better virErrorNumber values.

Signed-off-by: Pino Toscano 
---
Changes in v2:
- remove some of the VIR_ERR_INTERNAL_ERROR -> VIR_ERR_INVALID_ARG
  changes as they refer to really internal bits

 src/esx/esx_network_driver.c|  4 ++--
 src/esx/esx_storage_backend_iscsi.c |  4 ++--
 src/esx/esx_storage_backend_vmfs.c  | 12 ++--
 src/esx/esx_util.c  |  2 +-
 src/esx/esx_vi.c|  2 +-
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index d46fc9253d..88843aae2f 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -355,7 +355,7 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml)
 for (hostPortGroup = hostPortGroupList; hostPortGroup;
  hostPortGroup = hostPortGroup->_next) {
 if (STREQ(def->portGroups[i].name, hostPortGroup->spec->name)) 
{
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NETWORK_EXIST,
_("HostPortGroup with name '%s' exists 
already"),
def->portGroups[i].name);
 goto cleanup;
@@ -388,7 +388,7 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml)
 
 if (def->forward.ifs[i].type !=
 VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_SUPPORT,
_("unsupported device type in network %s "
  "interface pool"),
def->name);
diff --git a/src/esx/esx_storage_backend_iscsi.c 
b/src/esx/esx_storage_backend_iscsi.c
index 395a347cf3..017b800f06 100644
--- a/src/esx/esx_storage_backend_iscsi.c
+++ b/src/esx/esx_storage_backend_iscsi.c
@@ -321,7 +321,7 @@ esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned 
int flags)
 
 if (!target) {
 /* pool not found */
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_STORAGE_POOL,
_("Could not find storage pool with name '%s'"),
pool->name);
 goto cleanup;
@@ -699,7 +699,7 @@ esxStorageVolGetXMLDesc(virStorageVolPtr volume,
 }
 
 if (!scsiLun) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_STORAGE_VOL,
_("Could find volume with name: %s"), volume->name);
 goto cleanup;
 }
diff --git a/src/esx/esx_storage_backend_vmfs.c 
b/src/esx/esx_storage_backend_vmfs.c
index 4f613bf93b..a98001d6b2 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -897,7 +897,7 @@ esxStorageVolCreateXML(virStoragePoolPtr pool,
 goto cleanup;
 
 if (def->type != VIR_STORAGE_VOL_FILE) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+virReportError(VIR_ERR_NO_SUPPORT, "%s",
_("Creating non-file volumes is not supported"));
 goto cleanup;
 }
@@ -913,7 +913,7 @@ esxStorageVolCreateXML(virStoragePoolPtr pool,
 }
 
 if (!virStringHasCaseSuffix(def->name, ".vmdk")) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_SUPPORT,
_("Volume name '%s' has unsupported suffix, "
  "expecting '.vmdk'"), def->name);
 goto cleanup;
@@ -1032,7 +1032,7 @@ esxStorageVolCreateXML(virStoragePoolPtr pool,
 key = g_strdup(datastorePath);
 }
 } else {
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_SUPPORT,
_("Creation of %s volumes is not supported"),
virStorageFileFormatTypeToString(def->target.format));
 goto cleanup;
@@ -,7 +,7 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool,
 goto cleanup;
 
 if (def->type != VIR_STORAGE_VOL_FILE) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+virReportError(VIR_ERR_NO_SUPPORT, "%s",
_("Creating non-file volumes is not supported"));
 goto cleanup;
 }
@@ -1127,7 +1127,7 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool,
 }
 
 if (!virStringHasCaseSuffix(def->name, ".vmdk")) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_SUPPORT,
_("Volume name '%s' has unsupported suffix, "

[libvirt PATCH v2] esx: generator: fix free of elements in lists

2020-09-10 Thread Pino Toscano
When a list is freed, we iterate through all the items, invoking the
free function for each; the actual free function called for each element
is the function of the actual type of each element, and thus the @_next
pointer in the element struct has the same type as the element itself.
Currently, the free function gets the parent of the current element
type, and invoke its free function to continue freeing the list.
However, in case the hierarchy of the classes has more than 1 level
(i.e. Class <- SubClass <- SubSubClass), the invoked free function is
only the parent class' one, and not the actual base class of the
hierarchy.

To fix that, change the generator to get the base class of a class, and
invoking that instead.  Also, avoid to set the @_next back, as it is not
needed.

Fixes commits 5cff36e39ae691fbd7c40597df1732eecf294150 and
f76c6dde2e33233566e886d96e76b5fe0c102d9a.

Signed-off-by: Pino Toscano 
---
Changes in v2:
- ancestor -> base class

 scripts/esx_vi_generator.py | 26 +-
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/scripts/esx_vi_generator.py b/scripts/esx_vi_generator.py
index 863c8af964..e0782e35f3 100755
--- a/scripts/esx_vi_generator.py
+++ b/scripts/esx_vi_generator.py
@@ -751,13 +751,13 @@ class Object(GenericObject):
 source += "{\n"
 
 if self.features & Object.FEATURE__LIST:
-if self.extends is not None:
+base_class = get_base_class(self)
+if base_class:
 # avoid "dereferencing type-punned pointer will break
 # strict-aliasing rules" warnings
-source += "esxVI_%s *next = (esxVI_%s *)item->_next;\n\n" \
-  % (self.extends, self.extends)
-source += "esxVI_%s_Free();\n" % self.extends
-source += "item->_next = (esxVI_%s *)next;\n\n" % self.name
+source += "esxVI_%s *baseNext = (esxVI_%s 
*)item->_next;\n" \
+  % (base_class, base_class)
+source += "esxVI_%s_Free();\n\n" % base_class
 else:
 source += "esxVI_%s_Free(>_next);\n\n" % self.name
 
@@ -1250,6 +1250,21 @@ def is_known_type(type):
 type in enums_by_name)
 
 
+def get_base_class(obj):
+if not obj.extends:
+return None
+base_class = None
+try:
+base_class = base_class_by_name[obj.extends]
+except KeyError:
+parent = objects_by_name[obj.extends]
+base_class = get_base_class(parent)
+if not base_class:
+base_class = parent.name
+base_class_by_name[name] = base_class
+return base_class
+
+
 def open_file(filename):
 return open(filename, "wt")
 
@@ -1341,6 +1356,7 @@ managed_objects_by_name = {}
 enums_by_name = {}
 methods_by_name = {}
 block = None
+base_class_by_name = {}
 
 
 # parse input file
-- 
2.26.2



[libvirt PATCH] esx: improve some of the virErrorNumber used

2020-09-10 Thread Pino Toscano
A lot of virReportError() calls use VIR_ERR_INTERNAL_ERROR to represent
the number of the error, even in cases where there is one fitting more.
Hence, replace some of them with better virErrorNumber values.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_network_driver.c|  4 ++--
 src/esx/esx_storage_backend_iscsi.c |  4 ++--
 src/esx/esx_storage_backend_vmfs.c  | 12 +-
 src/esx/esx_util.c  |  4 ++--
 src/esx/esx_vi.c| 36 ++---
 5 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index d46fc9253d..88843aae2f 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -355,7 +355,7 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml)
 for (hostPortGroup = hostPortGroupList; hostPortGroup;
  hostPortGroup = hostPortGroup->_next) {
 if (STREQ(def->portGroups[i].name, hostPortGroup->spec->name)) 
{
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NETWORK_EXIST,
_("HostPortGroup with name '%s' exists 
already"),
def->portGroups[i].name);
 goto cleanup;
@@ -388,7 +388,7 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml)
 
 if (def->forward.ifs[i].type !=
 VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_SUPPORT,
_("unsupported device type in network %s "
  "interface pool"),
def->name);
diff --git a/src/esx/esx_storage_backend_iscsi.c 
b/src/esx/esx_storage_backend_iscsi.c
index 395a347cf3..017b800f06 100644
--- a/src/esx/esx_storage_backend_iscsi.c
+++ b/src/esx/esx_storage_backend_iscsi.c
@@ -321,7 +321,7 @@ esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned 
int flags)
 
 if (!target) {
 /* pool not found */
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_STORAGE_POOL,
_("Could not find storage pool with name '%s'"),
pool->name);
 goto cleanup;
@@ -699,7 +699,7 @@ esxStorageVolGetXMLDesc(virStorageVolPtr volume,
 }
 
 if (!scsiLun) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_STORAGE_VOL,
_("Could find volume with name: %s"), volume->name);
 goto cleanup;
 }
diff --git a/src/esx/esx_storage_backend_vmfs.c 
b/src/esx/esx_storage_backend_vmfs.c
index 4f613bf93b..a98001d6b2 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -897,7 +897,7 @@ esxStorageVolCreateXML(virStoragePoolPtr pool,
 goto cleanup;
 
 if (def->type != VIR_STORAGE_VOL_FILE) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+virReportError(VIR_ERR_NO_SUPPORT, "%s",
_("Creating non-file volumes is not supported"));
 goto cleanup;
 }
@@ -913,7 +913,7 @@ esxStorageVolCreateXML(virStoragePoolPtr pool,
 }
 
 if (!virStringHasCaseSuffix(def->name, ".vmdk")) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_SUPPORT,
_("Volume name '%s' has unsupported suffix, "
  "expecting '.vmdk'"), def->name);
 goto cleanup;
@@ -1032,7 +1032,7 @@ esxStorageVolCreateXML(virStoragePoolPtr pool,
 key = g_strdup(datastorePath);
 }
 } else {
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_SUPPORT,
_("Creation of %s volumes is not supported"),
virStorageFileFormatTypeToString(def->target.format));
 goto cleanup;
@@ -,7 +,7 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool,
 goto cleanup;
 
 if (def->type != VIR_STORAGE_VOL_FILE) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+virReportError(VIR_ERR_NO_SUPPORT, "%s",
_("Creating non-file volumes is not supported"));
 goto cleanup;
 }
@@ -1127,7 +1127,7 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool,
 }
 
 if (!virStringHasCaseSuffix(def->name, ".vmdk")) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_SUPPORT,
_("Volume name '%s' has unsupported suffix, "
  "expecting '.vmdk'"), def->name);
 goto cleanup;
@@ -

[libvirt PATCH] esx: generator: fix free of elements in lists

2020-09-10 Thread Pino Toscano
When a list is freed, we iterate through all the items, invoking the
free function for each; the actual free function called for each element
is the function of the actual type of each element, and thus the @_next
pointer in the element struct has the same type as the element itself.
Currently, the free function gets the parent of the current element
type, and invoke its free function to continue freeing the list.
However, in case the hierarchy of the classes has more than 1 level
(i.e. Class <- SubClass <- SubSubClass), the invoked free function is
only the parent class' one, and not the actual base class of the
hierarchy.

To fix that, change the generator to get the ancestor of a class, and
invoking that instead.  Also, avoid to set the @_next back, as it is not
needed.

Fixes commits 5cff36e39ae691fbd7c40597df1732eecf294150 and
f76c6dde2e33233566e886d96e76b5fe0c102d9a.

Signed-off-by: Pino Toscano 
---
 scripts/esx_vi_generator.py | 26 +-
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/scripts/esx_vi_generator.py b/scripts/esx_vi_generator.py
index 863c8af964..be9b3949e8 100755
--- a/scripts/esx_vi_generator.py
+++ b/scripts/esx_vi_generator.py
@@ -751,13 +751,13 @@ class Object(GenericObject):
 source += "{\n"
 
 if self.features & Object.FEATURE__LIST:
-if self.extends is not None:
+ancestor = get_ancestor(self)
+if ancestor:
 # avoid "dereferencing type-punned pointer will break
 # strict-aliasing rules" warnings
-source += "esxVI_%s *next = (esxVI_%s *)item->_next;\n\n" \
-  % (self.extends, self.extends)
-source += "esxVI_%s_Free();\n" % self.extends
-source += "item->_next = (esxVI_%s *)next;\n\n" % self.name
+source += "esxVI_%s *baseNext = (esxVI_%s 
*)item->_next;\n" \
+  % (ancestor, ancestor)
+source += "esxVI_%s_Free();\n\n" % ancestor
 else:
 source += "esxVI_%s_Free(>_next);\n\n" % self.name
 
@@ -1250,6 +1250,21 @@ def is_known_type(type):
 type in enums_by_name)
 
 
+def get_ancestor(obj):
+if not obj.extends:
+return None
+ancestor = None
+try:
+ancestor = ancestor_by_name[obj.extends]
+except KeyError:
+parent = objects_by_name[obj.extends]
+ancestor = get_ancestor(parent)
+if not ancestor:
+ancestor = parent.name
+ancestor_by_name[name] = ancestor
+return ancestor
+
+
 def open_file(filename):
 return open(filename, "wt")
 
@@ -1341,6 +1356,7 @@ managed_objects_by_name = {}
 enums_by_name = {}
 methods_by_name = {}
 block = None
+ancestor_by_name = {}
 
 
 # parse input file
-- 
2.26.2



[libvirt PATCH] gitdm: add missing aliases

2020-08-24 Thread Pino Toscano
Consider a couple of misspelt emails in B-y tags.

Signed-off-by: Pino Toscano 
---
 docs/gitdm/aliases | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/gitdm/aliases b/docs/gitdm/aliases
index 83637bdff9..467e90f141 100644
--- a/docs/gitdm/aliases
+++ b/docs/gitdm/aliases
@@ -1,6 +1,8 @@
 # Silly mistakes, mostly found in S-o-b or R-b tags.
 
+"berrange redhat com" berra...@redhat.com
 "jdenemar redhat com" jdene...@redhat.com
+"jtomko redhat com" jto...@redhat.com
 "pkrempa@redhat st.com" pkre...@redhat.com
 berrange@localhost.localdomain berra...@redhat.com
 jyang@redhat jy...@redhat.com
-- 
2.26.2



[libvirt PATCH] gitdm: move pld-linux.org to opensource

2020-08-24 Thread Pino Toscano
PLD Linux is a Linux distribution, so @pld-linux.org fits in the
opensource group with similar projects.

Signed-off-by: Pino Toscano 
---
 docs/gitdm/groups/opensource   | 1 +
 docs/gitdm/groups/unaffiliated | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/gitdm/groups/opensource b/docs/gitdm/groups/opensource
index 4ac81e7468..641c1ae3f4 100644
--- a/docs/gitdm/groups/opensource
+++ b/docs/gitdm/groups/opensource
@@ -8,6 +8,7 @@ gnu.org
 kernel.org
 linux.com
 openbsd.org
+pld-linux.org
 salasaga.org
 samba.org
 sdf.org
diff --git a/docs/gitdm/groups/unaffiliated b/docs/gitdm/groups/unaffiliated
index 6e9de7084a..5f867fc5e4 100644
--- a/docs/gitdm/groups/unaffiliated
+++ b/docs/gitdm/groups/unaffiliated
@@ -33,7 +33,6 @@ alexander.nu...@nfvexpress.com
 and...@lagarcavilla.org
 and...@interpretmath.pw
 asad.sa...@acidseed.com
-at...@pld-linux.org
 ben...@dolka.fr
 be...@binaries.fr
 bi...@bigon.be
-- 
2.26.2



Re: [PATCH 1/2] virdevmapper: Don't error on kernels without DM support

2020-08-18 Thread Pino Toscano
t first. You can look it
> >up and cache it after you open the socket and don't ever need to re-do
> >it. In that case you can also use the existing VIR_ONCE code.
> > 
> >You then don't have to clear it when the module is ejected, because
> >afterwards the control socket will not exist. In case the module is
> >re-injected, given the contstraints above the value will not change so
> >no need to re-load.
> > 
> > 2) at module insertion time
> > 
> >In this case it's actually wrong to cache it, because the module can
> >be unloaded and reloaded while libvirt will not check and update the
> >cached value. In those scenarios it should also be determined only
> >after you open the control fd frist.
> > 
> As promised yesterday, I've dived into the code and found out that the 
> major number can be specified as a parameter to the dm module (just 
> tested and it works). So the next thing I tried was to see how could we 
> check whether the module was reloaded. I've tried opening 
> /dev/mapper/control hoping that I will get EOF on module unload (which I 
> could then use to run a callback that would invalidate the cached 
> value). But having the file open prevents unloading the module.
> 
> Loading/unloading a module results in an udev event, BUT we listen for 
> udev events only in nodedev driver and moving the code out to a driver 
> agnostic location and making it driver agnostic is too much code for a 
> little gain.
> 
> Then I looked whether it's possible to get the major number via an 
> ioctl(). But haven't found anything.

What about stat()ing /dev/mapper/control? That should give you the
major/minor of that special character device.

-- 
Pino Toscano

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


[libvirt PATCH] docs: improve auth service listing

2020-08-17 Thread Pino Toscano
Slightly improve the list of known authentication service types:
- reword 'ssh' to mention it is used for the ssh driver (for remote
  QEMU), and stop mentioning the removed Phyp driver
- add 'hyperv', used by the HyperV driver
- alphabetically sort the list
- use a bulletted list instead of a numbered one

Signed-off-by: Pino Toscano 
---
 docs/auth.html.in | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/docs/auth.html.in b/docs/auth.html.in
index 6f8805b6f5..9964313776 100644
--- a/docs/auth.html.in
+++ b/docs/auth.html.in
@@ -122,17 +122,19 @@ credentials=defgrp
 
 
 
-  The following service types are known to libvirt
+  The following service types are known to libvirt:
 
 
-
-  libvirt - used for connections to a libvirtd
-server, which is configured with SASL auth
-  ssh - used for connections to a Phyp server
-over SSH, but the Phyp driver has been removed
+
   esx - used for connections to an ESX or
 VirtualCenter server
-
+  hyperv - used for connections to an HyperV
+server
+  libvirt - used for connections to a libvirtd
+server, which is configured with SASL auth
+  ssh - used for connections to a remote QEMU driver
+over SSH
+
 
 
   Applications using libvirt are free to use this same configuration
-- 
2.26.2



Re: [libvirt PATCH] tests: fix license blurb in virsh-undefine

2020-08-05 Thread Pino Toscano
On Tuesday, 4 August 2020 20:28:06 CEST Ján Tomko wrote:
> Assume commit 0466ff28f2 used case-insensitive replace s/OUT/EXP/
> by mistake and this file is still licensed under GPLv2.0+
> 
> Undo the change.
> 
> Signed-off-by: Ján Tomko 
> FIxes: 0466ff28f23f4c430906efd5859f87672cf08782
> Cc: Cole Robinson 
> Cc: Eric Blake 
> Cc: Pino Toscano 
> ---

Reviewed-by: Pino Toscano 

-- 
Pino Toscano

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


Re: [libvirt PATCH 312/351] meson: tests: add file access test setup

2020-07-28 Thread Pino Toscano
On Tuesday, 28 July 2020 14:56:45 CEST Peter Krempa wrote:
> On Thu, Jul 16, 2020 at 11:59:08 +0200, Pavel Hrdina wrote:
> > We need to modify check-file-access.py to be usable as wrapper for
> > libvirt tests. This way we can run the tests using this command:
> > 
> > meson test --setup access
> > 
> > which will run all tests using check-file-access.py as a wrapper.
> > 
> > With autotools all file access are written into single file for all
> > tests and compared once the whole test suite is done.
> > 
> > With Meson we will compare the file access after every single test
> > because it is used as wrapper now. That requires writing the file
> > access into separate files for every single test as they are executed
> > in parallel.
> > 
> > Since the wrapper is used for all tests in Meson including tests outside
> > of tests directory we have to check for presence of the output file.
> > We should also cleanup after ourselves.
> > 
> > Signed-off-by: Pavel Hrdina 
> > ---
> >  Makefile.am  |  3 ---
> >  scripts/check-file-access.py | 24 +++-
> >  tests/Makefile.am| 12 
> >  tests/meson.build|  8 
> >  tests/virtestmock.c  |  2 +-
> >  5 files changed, 28 insertions(+), 21 deletions(-)
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 363c5cf66fd..d05a0c1a85a 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -37,9 +37,6 @@ srpm: clean
> >  
> >  check-local: all tests
> >  
> > -check-access: all
> > -   @($(MAKE) $(AM_MAKEFLAGS) -C tests check-access)
> > -
> >  dist-hook: gen-AUTHORS
> >  
> >  .PHONY: gen-AUTHORS
> > diff --git a/scripts/check-file-access.py b/scripts/check-file-access.py
> > index aa120cafacf..f0e98f4b652 100755
> > --- a/scripts/check-file-access.py
> > +++ b/scripts/check-file-access.py
> > @@ -21,15 +21,27 @@
> >  #
> >  #
> >  
> > +import os
> > +import random
> >  import re
> > +import string
> >  import sys
> >  
> > -if len(sys.argv) != 3:
> > -print("syntax: %s ACCESS-FILE PERMITTED-ACCESS-FILE")
> > -sys.exit(1)
> > +abs_builddir = os.environ.get('abs_builddir', '')
> > +abs_srcdir = os.environ.get('abs_srcdir', '')
> >  
> > -access_file = sys.argv[1]
> > -permitted_file = sys.argv[2]
> > +filename = ''.join(random.choice(string.ascii_letters) for _ in range(16))
> 
> Umm, python doesn't have a 'mkostemp' equivalent? This seems a bit
> fishy.

Sure, it is the tempfile module:
https://docs.python.org/3/library/tempfile.html

  filename = tempfile.mkstemp(dir=abs_builddir, prefix='file-access-', 
suffix='.txt')

-- 
Pino Toscano

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


Re: [libvirt PATCH 3/3] conf: scheduler parser: do not hardcode element name

2020-07-27 Thread Pino Toscano
On Monday, 27 July 2020 15:15:46 CEST Ján Tomko wrote:
> When trying to parse an XML with overlapping iothread scheduler
> settings, the error message was rather confusing:
> 
>error: iothreadssched attributes 'vcpus' must not overlap
> 
> Pass the correct element name.
> 
> Signed-off-by: Ján Tomko 
> ---
>  src/conf/domain_conf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 0f12b54575..1179a27a00 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -20024,8 +20024,8 @@ virDomainThreadSchedParseHelper(xmlNodePtr node,
>  
>  if (sched->policy != VIR_PROC_POLICY_NONE) {
>  virReportError(VIR_ERR_XML_DETAIL,
> -   _("'%s' attributes 'vcpus' must not overlap"),
> -   elementName);
> +   _("'%s' attributes '%s' must not overlap"),
> +   elementName, attributeName);

While I generally agree with this kind of changes, please note that
this is difficult in C: see:
https://www.gnu.org/software/gettext/manual/gettext.html#c_002dformat

-- 
Pino Toscano

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


Re: [PATCH V2] qemu: pre-create the dbus directory in qemuStateInitialize

2020-07-21 Thread Pino Toscano
On Wednesday, 22 July 2020 04:09:24 CEST Bihong Yu wrote:
> +if (virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
> + VIR_DIR_CREATE_ALLOW_EXIST) < 0) {
> +virReportSystemError(errno, _("Failed to create dbus state dir %s"),
> + cfg->dbusStateDir);

Minor notes on the message:
- spell "D-Bus" correctly
- no need to abbreviate "directory"
- quote the path placeholder
so I suggest something like:
  "Failed to create the D-Bus state directory '%s'"

(Can't comment on the rest of the changes, sorry.)

-- 
Pino Toscano

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


[libvirt PATCH] docs: virConnectGetCapabilities do not provide pool types

2020-07-20 Thread Pino Toscano
Remove the paragraph in the storage pool page that mentions
virConnectGetCapabilities, as virConnectGetCapabilities does not return
any information about pools.

Signed-off-by: Pino Toscano 
---
 docs/formatstoragecaps.html.in | 6 --
 1 file changed, 6 deletions(-)

diff --git a/docs/formatstoragecaps.html.in b/docs/formatstoragecaps.html.in
index ee3888f44d..d8a1cacd96 100644
--- a/docs/formatstoragecaps.html.in
+++ b/docs/formatstoragecaps.html.in
@@ -13,12 +13,6 @@
 supported, and if relevant the source format types, the required
 source elements, and the target volume format types. 
 
-The Storage Pool Capabilities XML provides more information than the
-  
-virConnectGetCapabilities
-  
-which only provides an enumerated list of supported pool types.
-
 Element and attribute overview
 
 A query interface was added to the virConnect API's to retrieve the
-- 
2.26.2



Format/style of UI message

2020-07-17 Thread Pino Toscano
Hi,

I recently took a look at the UI/user visible messages from libvirt,
which are translated using gettext. They are extracted in a single
libvirt.pot catalog, which includes messages from libvirt.so itself
(mostly, if not all, errors), the separate daemons, the helper tools,
and from virsh.

I noticed there is plently of room for improvements: what strikes is
the lack of consistency among the messages. Let me state first: I
understand that not all the people are native English speakers
(I am not), so I'm not picking against anyone.

Some examples:

a) different capitalization:
- "cannot open %s"
- "Cannot open %s"

b) different quoting for files/identifiers/etc:
- "Cannot open %s"
- "Cannot open '%s'"

c) different verbs for failed actions:
- "Cannot frobnicate ..."
- "Could not frobnicate ..."
- "Did not frobnicate ..."
- "Failed to frobnicate ..."
- "Unable to frobnicate ..."
depending on the message, also "frobbing failed"

d) sometimes contractions ("couldn't", "don't", etc), sometimes not
("could not", "do not", etc)

e) what QEMU/etc supports:
- "... by this QEMU binary"
- "... for this QEMU binary"
- "... in this QEMU binary"
- "... with this QEMU binary"
- "... by this QEMU"
- "... for this QEMU"
- "... with this QEMU"
- "... with this binary" [in a QEMU file]
- "... [supported] by qemu"
there is also "qemu does not support ...", which I think it can stay
for now; also both "available [by/for/etc]" and "supported [by/for/etc]"
are used

I can give it a try in fixing the messages to be more consistent all
around; before I start the mass editing, I need to know which style to
follow:

a) it seems like the virError fields @message, @str1, @str2 and @str3
are joined together in reporting/log strings like "error: ";
hence, should they be not capitalized? It may look OK in English, but
less nice and hard to fix in translations.
Obviously, sentences as shown in tools (e.g. virsh) definitely need to
be properly capitalized.

b) should identifiers such as filenames, paths, XML tags, JSON fields,
etc be always quoted?

c) which verb to use when something failed? "could not" is a subjective
thing, not a past action; "failed" seems to imply that something was
attempted; "did not" seems to imply that it was not done, but nothing
whether it was attempted; the rest sort of indicate the ability to do
something.

d) allow contractions or not? They are generally used in spoken/informal
language, and while libvirt is not that formal it should not be that
colloquial either IMHO; also, they make the text slightly harder to
understand by non-native speakers, and they are lost when translating.
A POV on the matter is:
https://www.businesswritingblog.com/business_writing/2006/04/dont_use_contra.html

e) which message to use to indicate that QEMU does not support
something?

Thanks,
-- 
Pino Toscano

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


Re: [PATCH trivial 1/1] logging.html.in: fix number of output formats available

2020-05-19 Thread Pino Toscano
On Tuesday, 19 May 2020 21:55:19 CEST Daniel Henrique Barboza wrote:
> There are 4 formats available (x:stderr, x:syslog:name,
> x:file:file_path, x:journald), not 3.
> 
> Signed-off-by: Daniel Henrique Barboza 
> ---
>  docs/logging.html.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/docs/logging.html.in b/docs/logging.html.in
> index 65c13e8a19..9f320b439e 100644
> --- a/docs/logging.html.in
> +++ b/docs/logging.html.in
> @@ -124,7 +124,7 @@ x:name
> priority level, messages that match that filter will still be logged,
> while others will not. In order to see those messages, you must also 
> have
> an output defined that includes the priority level of your filter.
> -The format for an output can be one of those 3 forms:
> +The format for an output can be one of those 4 forms:

Or maybe s/those 3/the following/, so there is no need to keep the
updated count of the allowed forms (I think it doesn't matter how many
they are).

-- 
Pino Toscano

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


Re: [libvirt PATCH] qemu: reject readonly attribute for virtiofs

2020-05-13 Thread Pino Toscano
On Wednesday, 13 May 2020 10:58:51 CEST Ján Tomko wrote:
> +if (fs->readonly) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("virtiofs does not yet supported read-only 
> mode"));

s/supported/support/

-- 
Pino Toscano

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


[PATCH ocaml 0/1] RFC: add/switch to dune build system

2020-04-14 Thread Pino Toscano
ocaml-libvirt is currently built using manually written makefiles, and
an autoconf script to detect libvirt (using pkg-config). While this
seems OK, it has a lot of downsides:
- all the OCaml rules are manually written
- there is some form of duplication in the OCaml rules
- the bytecode vs native build is fragile

Initially named jhbuild, dune is a native OCaml build system which plans
to eventually replace the others (omake, oasis, ocamlbuild), with a
growing user base in the OCaml community.

This is a very simple patch adding support for dune. The idea is to
replace the current autoconf+makefile build system, which can be
removed more or less easily. I'd prefer to not maintain two build
systems at once, so this patch is also a way to start a discussion about
the proposed approach.

Things not yet done (pending the discussion):
- rewrite/simplify the release bits, currently interwonen in the
  makefiles
- update the documentation
- update libvirt-ci
- remove the autoconf+makefile build system

Pino Toscano (1):
  Add dune build system

 .gitignore |  3 +++
 dune   | 18 +
 dune-project   |  1 +
 dune.inc   | 22 
 examples/dune  | 50 
 libvirt/discover.ml| 40 +
 libvirt/dune   | 57 ++
 libvirt/dune-project   |  2 ++
 libvirt/mllibvirt.opam | 33 
 9 files changed, 226 insertions(+)
 create mode 100644 dune
 create mode 100644 dune-project
 create mode 100644 dune.inc
 create mode 100644 examples/dune
 create mode 100644 libvirt/discover.ml
 create mode 100644 libvirt/dune
 create mode 100644 libvirt/dune-project
 create mode 100644 libvirt/mllibvirt.opam

-- 
2.25.2



[PATCH ocaml 1/1] Add dune build system

2020-04-14 Thread Pino Toscano
Introduce the dune build system to build ocaml-libvirt, providing
everything that the current autoconf-based build system has.

Add also an opam configuration files, as it is mandatory with dune.

Signed-off-by: Pino Toscano 
---
 .gitignore |  3 +++
 dune   | 18 +
 dune-project   |  1 +
 dune.inc   | 22 
 examples/dune  | 50 
 libvirt/discover.ml| 40 +
 libvirt/dune   | 57 ++
 libvirt/dune-project   |  2 ++
 libvirt/mllibvirt.opam | 33 
 9 files changed, 226 insertions(+)
 create mode 100644 dune
 create mode 100644 dune-project
 create mode 100644 dune.inc
 create mode 100644 examples/dune
 create mode 100644 libvirt/discover.ml
 create mode 100644 libvirt/dune
 create mode 100644 libvirt/dune-project
 create mode 100644 libvirt/mllibvirt.opam

diff --git a/.gitignore b/.gitignore
index c52c1b8..814457b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,10 +10,12 @@
 *.opt
 *.so
 *~
+.merlin
 Make.rules
 Makefile
 core
 core.*
+/_build/
 /META
 /aclocal.m4
 /autom4te.cache
@@ -30,5 +32,6 @@ core.*
 /html/
 /libvirt/libvirt_generated.c
 /libvirt/libvirt_version.ml
+/libvirt/mllibvirt.install
 /ocaml-libvirt-*.exe
 /ocaml-libvirt-*.tar.gz
diff --git a/dune b/dune
new file mode 100644
index 000..5002345
--- /dev/null
+++ b/dune
@@ -0,0 +1,18 @@
+; OCaml bindings for libvirt.
+; (C) Copyright 2020 Pino Toscano, Red Hat Inc.
+;
+; This library is free software; you can redistribute it and/or
+; modify it under the terms of the GNU Lesser General Public
+; License as published by the Free Software Foundation; either
+; version 2 of the License, or (at your option) any later version.
+;
+; This library is distributed in the hope that it will be useful,
+; but WITHOUT ANY WARRANTY; without even the implied warranty of
+; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+; Lesser General Public License for more details.
+;
+; You should have received a copy of the GNU Lesser General Public
+; License along with this library; if not, write to the Free Software
+; Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301  USA
+
+(include dune.inc)
diff --git a/dune-project b/dune-project
new file mode 100644
index 000..de4fc20
--- /dev/null
+++ b/dune-project
@@ -0,0 +1 @@
+(lang dune 1.0)
diff --git a/dune.inc b/dune.inc
new file mode 100644
index 000..2cfb767
--- /dev/null
+++ b/dune.inc
@@ -0,0 +1,22 @@
+; OCaml bindings for libvirt.
+; (C) Copyright 2020 Pino Toscano, Red Hat Inc.
+;
+; This library is free software; you can redistribute it and/or
+; modify it under the terms of the GNU Lesser General Public
+; License as published by the Free Software Foundation; either
+; version 2 of the License, or (at your option) any later version.
+;
+; This library is distributed in the hope that it will be useful,
+; but WITHOUT ANY WARRANTY; without even the implied warranty of
+; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+; Lesser General Public License for more details.
+;
+; You should have received a copy of the GNU Lesser General Public
+; License along with this library; if not, write to the Free Software
+; Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301  USA
+
+(env
+  (dev
+(flags (:standard -g -warn-error CDEFLMPSUVYZX-3 -w -9-27-34-37-50 
-safe-string)))
+  (release
+(flags (:standard -g -warn-error CDEFLMPSUVYZX-3 -w -9-27-34-37-50 
-safe-string
diff --git a/examples/dune b/examples/dune
new file mode 100644
index 000..219727c
--- /dev/null
+++ b/examples/dune
@@ -0,0 +1,50 @@
+; OCaml bindings for libvirt.
+; (C) Copyright 2020 Pino Toscano, Red Hat Inc.
+;
+; This library is free software; you can redistribute it and/or
+; modify it under the terms of the GNU Lesser General Public
+; License as published by the Free Software Foundation; either
+; version 2 of the License, or (at your option) any later version.
+;
+; This library is distributed in the hope that it will be useful,
+; but WITHOUT ANY WARRANTY; without even the implied warranty of
+; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+; Lesser General Public License for more details.
+;
+; You should have received a copy of the GNU Lesser General Public
+; License along with this library; if not, write to the Free Software
+; Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301  USA
+
+(executable
+  (name domain_events)
+  (modules Domain_events)
+  (libraries mllibvirt))
+
+(executable
+  (name get_all_domain_stats)
+  (modules Get_all_domain_stats)
+  (libraries mllibvirt unix))
+
+(executable
+  (name get_cpu_stats)
+  (modules Get_cpu_stats)
+  (libraries mllibvirt))
+
+(executable
+  (name list_domains)
+  (modules List_domains)
+  (libraries mllibvirt))
+
+(executable

Re: [PATCH 05/43] esx: convert virMutex to GMutex

2020-04-14 Thread Pino Toscano
On Friday, 10 April 2020 15:54:32 CEST Rafael Fonseca wrote:
> @@ -346,11 +342,12 @@ esxStreamClose(virStreamPtr stream, bool finish)
>  {
>  int result = 0;
>  esxStreamPrivate *priv = stream->privateData;
> +g_autoptr(GMutexLocker) locker = NULL;
>  
>  if (!priv)
>  return 0;
>  
> -virMutexLock(>curl->lock);
> +locker = g_mutex_locker_new(>curl->lock);
>  
>  if (finish && priv->backlog_used > 0) {
>  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -360,8 +357,6 @@ esxStreamClose(virStreamPtr stream, bool finish)
>  
>  stream->privateData = NULL;
>  
> -virMutexUnlock(>curl->lock);
> -
>  esxFreeStreamPrivate();

Careful here, this is a problematic situation:
- the lock is indirectly part of the @priv structure
- esxFreeStreamPrivate calls esxVI_CURL_Free(priv->curl)
- esxVI_CURL_Free calls virMutexDestroy(>lock)
- lock is still locked, so it will deadlock (or crash, or something
  not good anyway)

You must unlock the mutex before esxFreeStreamPrivate is called.

I did not check other patches of this long series for similar potential
issues, please do check them.

> diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
> index 16690edfbe..ed6c6c28cd 100644
> --- a/src/esx/esx_vi.c
> +++ b/src/esx/esx_vi.c
> @@ -433,14 +429,13 @@ int
>  esxVI_CURL_Upload(esxVI_CURL *curl, const char *url, const char *content)
>  {
>  int responseCode = 0;
> +g_autoptr(GMutexLocker) locker = g_mutex_locker_new(>lock);
>  
>  if (!content) {
>  virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
>  return -1;
>  }
>  
> -virMutexLock(>lock);
> -

Careful #2 here about locking earlier: while usually this is not an
issue, it could be in case the code that was executed without the lock
held can be called by other code branches with the lock held.

Again, this must be thoroughly checked in the whole patch series.

-- 
Pino Toscano

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


Re: [PATCH ocaml] build: skip github lockdown file in manifest check

2020-04-07 Thread Pino Toscano
On Tuesday, 7 April 2020 20:14:44 CEST Daniel P. Berrangé wrote:
> The github lockdown file is not intended to be included in any built
> dist, so should be excluded when checking the manifest.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
> 
> Pushed as a CI build fix
> 
>  Makefile.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile.in b/Makefile.in
> index 4357ba6..54ecaf3 100644
> --- a/Makefile.in
> +++ b/Makefile.in
> @@ -86,7 +86,7 @@ dist: ChangeLog
>   ls -l $(PACKAGE)-$(VERSION).tar.gz
>  
>  check-manifest:
> - git ls-files > .check-manifest; \
> + git ls-files | grep -v .github/lockdown.yml > .check-manifest; \

Thanks, this seems OK.

Maybe one small improvement if you have the time/will: skip the entire
.github/ toplevel directory, so in case we add more files there it will
not require any change.

Thanks,
-- 
Pino Toscano

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


Re: [PATCH 5/6] virprocess: Passthru error from virProcessRunInForkHelper

2020-03-24 Thread Pino Toscano
On Tuesday, 24 March 2020 11:48:17 CET Daniel P. Berrangé wrote:
> On Wed, Mar 18, 2020 at 06:32:15PM +0100, Michal Privoznik wrote:
> > When running a function in a forked child, so far the only thing
> > we could report is exit status of the child and the error
> > message. However, it may be beneficial to the caller to know the
> > actual error that happened in the child.
> > 
> > Signed-off-by: Michal Privoznik 
> > ---
> >  build-aux/syntax-check.mk |  2 +-
> >  src/util/virprocess.c | 51 ---
> >  tests/commandtest.c   | 43 +
> >  3 files changed, 91 insertions(+), 5 deletions(-)
> > diff --git a/tests/commandtest.c b/tests/commandtest.c
> > index a64aa9ad33..f4a2c67c05 100644
> > --- a/tests/commandtest.c
> > +++ b/tests/commandtest.c
> > @@ -1257,6 +1257,48 @@ static int test27(const void *unused G_GNUC_UNUSED)
> >  }
> >  
> >  
> > +static int
> > +test28Callback(pid_t pid G_GNUC_UNUSED,
> > +   void *opaque G_GNUC_UNUSED)
> > +{
> > +virReportSystemError(ENODATA, "%s", "some error message");
> > +return -1;
> > +}
> > +
> > +
> > +static int
> > +test28(const void *unused G_GNUC_UNUSED)
> > +{
> > +/* Not strictly a virCommand test, but this is the easiest place
> > + * to test this lower-level interface. */
> > +virErrorPtr err;
> > +
> > +if (virProcessRunInFork(test28Callback, NULL) != -1) {
> > +fprintf(stderr, "virProcessRunInFork did not fail\n");
> > +return -1;
> > +}
> > +
> > +if (!(err = virGetLastError())) {
> > +fprintf(stderr, "Expected error but got nothing\n");
> > +return -1;
> > +}
> > +
> > +if (!(err->code == VIR_ERR_SYSTEM_ERROR &&
> > +  err->domain == 0 &&
> > +  STREQ(err->message, "some error message: No data available") &&
> > +  err->level == VIR_ERR_ERROR &&
> > +  STREQ(err->str1, "%s") &&
> > +  STREQ(err->str2, "some error message: No data available") &&
> > +  err->int1 == ENODATA &&
> > +  err->int2 == -1)) {
> > +fprintf(stderr, "Unexpected error object\n");
> > +return -1;
> > +}
> 
> This new test fails on FreeBSD
> 
> $ VIR_TEST_DEBUG=1 VIR_TEST_RANGE=28 ./commandtest 
> TEST: commandtest
> 28) Command Exec test28 test  ... 
> Unexpected error object
> libvirt:  error : some error message: Input/output error
> FAILED
> 
> 
> IIUC the problem here is that the STREQ check is assuming that the
> ENODATA errno results in the string "No data available". The strings
> are not standardized by POSIX AFAIK, so C libraries can use any reasonable
> text for them. So this check is not portable.
> 
> Perhaps its enough to use  STRPREFIX(err->str2, "some error message:") ?

Or maybe use g_strerror to get the error message of ENODATA, and
compare it to the actual message got in the test.

-- 
Pino Toscano

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


[PATCH] tests: switch away from HAVE_SOCKETPAIR

2020-03-19 Thread Pino Toscano
Since the removal of gnulib, HAVE_SOCKETPAIR is no more defined, making
these two tests effectively skipped.

Use the same strategy used in other generic library bits, i.e. exclude
the socketpair usage on Windows.

Semi-related change in virnetdaemontest.c to make it build: since
virutil.h does not include unistd.h anymore, we need to include it.

Signed-off-by: Pino Toscano 
---
 tests/virnetdaemontest.c   | 4 +++-
 tests/virnetserverclienttest.c | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c
index 825487f0a1..09d268627c 100644
--- a/tests/virnetdaemontest.c
+++ b/tests/virnetdaemontest.c
@@ -18,13 +18,15 @@
 
 #include 
 
+#include 
+
 #include "testutils.h"
 #include "virerror.h"
 #include "rpc/virnetdaemon.h"
 
 #define VIR_FROM_THIS VIR_FROM_RPC
 
-#if defined(HAVE_SOCKETPAIR) && defined(WITH_YAJL)
+#if !defined(WIN32) && defined(WITH_YAJL)
 struct testClientPriv {
 int magic;
 };
diff --git a/tests/virnetserverclienttest.c b/tests/virnetserverclienttest.c
index a9a56c48d5..668fd02a1e 100644
--- a/tests/virnetserverclienttest.c
+++ b/tests/virnetserverclienttest.c
@@ -24,7 +24,7 @@
 
 #define VIR_FROM_THIS VIR_FROM_RPC
 
-#ifdef HAVE_SOCKETPAIR
+#ifndef WIN32
 
 static void *
 testClientNew(virNetServerClientPtr client G_GNUC_UNUSED,
-- 
2.25.1



Re: [PATCH] vmx: make 'fileName' optional for CD-ROMs

2020-03-18 Thread Pino Toscano
(Apparently forgot to send it yesterday, so sending it with a small
addedum.)

On Tuesday, 17 March 2020 18:09:04 CET Richard W.M. Jones wrote:
> My only worry about this patch is that it relies on fileName now
> possibly being NULL, which means if there is any case that you've
> missed now — or one added in future — which doesn't consider that
> fileName might be NULL then it'll crash (libvirtd? or virsh? I'm not
> sure).

In case now (even in v2) fileName is used without checking, it will
crash libvirt, as the esx/vmware drivers are built-in in the library.

> I wonder if therefore it would be safer to set the string to a
> known-good non-NULL value such as ‘strdup ("emptyBackingString")’?

Thought about that, however "emptyBackingString" seems like a magic
identifier, and it would be trading one hack with another, somehow.

-- 
Pino Toscano

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


[PATCH v2 1/2] vmx: shortcut earlier few 'ignore' cases in virVMXParseDisk()

2020-03-18 Thread Pino Toscano
Move earlier the checks for skipping a hard disk when parsing a CD-DROM,
and for skipping a CD-ROM when parsing a hard disk. This should have no
behaviour changes, and avoids to add repeated checks in following
commits.

Signed-off-by: Pino Toscano 
---
 src/vmx/vmx.c | 48 
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index f0140129a2..bfc9bc7404 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2218,7 +2218,21 @@ virVMXParseDisk(virVMXContext *ctx, 
virDomainXMLOptionPtr xmlopt, virConfPtr con
 
 /* Setup virDomainDiskDef */
 if (device == VIR_DOMAIN_DISK_DEVICE_DISK) {
-if (virStringHasCaseSuffix(fileName, ".vmdk")) {
+if (virStringHasCaseSuffix(fileName, ".iso") ||
+STREQ(fileName, "emptyBackingString") ||
+(deviceType &&
+ (STRCASEEQ(deviceType, "atapi-cdrom") ||
+  STRCASEEQ(deviceType, "cdrom-raw") ||
+  (STRCASEEQ(deviceType, "scsi-passthru") &&
+   STRPREFIX(fileName, "/vmfs/devices/cdrom/") {
+/*
+ * This function was called in order to parse a harddisk device,
+ * but .iso files, 'atapi-cdrom', 'cdrom-raw', and 'scsi-passthru'
+ * CDROM devices are for CDROM devices only. Just ignore it, 
another
+ * call to this function to parse a CDROM device may handle it.
+ */
+goto ignore;
+} else if (virStringHasCaseSuffix(fileName, ".vmdk")) {
 char *tmp;
 
 if (deviceType != NULL) {
@@ -2254,20 +2268,6 @@ virVMXParseDisk(virVMXContext *ctx, 
virDomainXMLOptionPtr xmlopt, virConfPtr con
 if (mode)
 (*def)->transient = STRCASEEQ(mode,
   "independent-nonpersistent");
-} else if (virStringHasCaseSuffix(fileName, ".iso") ||
-   STREQ(fileName, "emptyBackingString") ||
-   (deviceType &&
-(STRCASEEQ(deviceType, "atapi-cdrom") ||
- STRCASEEQ(deviceType, "cdrom-raw") ||
- (STRCASEEQ(deviceType, "scsi-passthru") &&
-  STRPREFIX(fileName, "/vmfs/devices/cdrom/") {
-/*
- * This function was called in order to parse a harddisk device,
- * but .iso files, 'atapi-cdrom', 'cdrom-raw', and 'scsi-passthru'
- * CDROM devices are for CDROM devices only. Just ignore it, 
another
- * call to this function to parse a CDROM device may handle it.
- */
-goto ignore;
 } else {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid or not yet handled value '%s' "
@@ -2277,7 +2277,15 @@ virVMXParseDisk(virVMXContext *ctx, 
virDomainXMLOptionPtr xmlopt, virConfPtr con
 goto cleanup;
 }
 } else if (device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
-if (virStringHasCaseSuffix(fileName, ".iso")) {
+if (virStringHasCaseSuffix(fileName, ".vmdk")) {
+/*
+ * This function was called in order to parse a CDROM device, but
+ * .vmdk files are for harddisk devices only. Just ignore it,
+ * another call to this function to parse a harddisk device may
+ * handle it.
+ */
+goto ignore;
+} else if (virStringHasCaseSuffix(fileName, ".iso")) {
 char *tmp;
 
 if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) {
@@ -2295,14 +2303,6 @@ virVMXParseDisk(virVMXContext *ctx, 
virDomainXMLOptionPtr xmlopt, virConfPtr con
 goto cleanup;
 }
 VIR_FREE(tmp);
-} else if (virStringHasCaseSuffix(fileName, ".vmdk")) {
-/*
- * This function was called in order to parse a CDROM device, but
- * .vmdk files are for harddisk devices only. Just ignore it,
- * another call to this function to parse a harddisk device may
- * handle it.
- */
-goto ignore;
 } else if (deviceType && STRCASEEQ(deviceType, "atapi-cdrom")) {
 virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK);
 
-- 
2.25.1



[PATCH v2 2/2] vmx: make 'fileName' optional for CD-ROMs

2020-03-18 Thread Pino Toscano
It seems like CD-ROMs may have no 'fileName' property specified in case
there is nothing configured as attachment for the drive. Hence, make
sure that virVMXParseDisk() do not consider it mandatory anymore,
considering it an empty block cdrom device. Sadly virVMXParseDisk() is
used also to parse disk and floppies, so make sure that a NULL fileName
is handled in cdrom- and floppy-related paths.

https://bugzilla.redhat.com/show_bug.cgi?id=1808610

Signed-off-by: Pino Toscano 
---
 src/vmx/vmx.c | 25 ++-
 .../vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx |  4 +++
 .../vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml | 23 +
 tests/vmx2xmltest.c   |  1 +
 4 files changed, 41 insertions(+), 12 deletions(-)
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index bfc9bc7404..6c6ef7acf3 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2207,7 +2207,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 goto cleanup;
 
 /* vmx:fileName -> def:src, def:type */
-if (virVMXGetConfigString(conf, fileName_name, , false) < 0)
+if (virVMXGetConfigString(conf, fileName_name, , true) < 0)
 goto cleanup;
 
 /* vmx:writeThrough -> def:cachemode */
@@ -2218,7 +2218,8 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 
 /* Setup virDomainDiskDef */
 if (device == VIR_DOMAIN_DISK_DEVICE_DISK) {
-if (virStringHasCaseSuffix(fileName, ".iso") ||
+if (fileName == NULL ||
+virStringHasCaseSuffix(fileName, ".iso") ||
 STREQ(fileName, "emptyBackingString") ||
 (deviceType &&
  (STRCASEEQ(deviceType, "atapi-cdrom") ||
@@ -2277,7 +2278,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 goto cleanup;
 }
 } else if (device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
-if (virStringHasCaseSuffix(fileName, ".vmdk")) {
+if (fileName && virStringHasCaseSuffix(fileName, ".vmdk")) {
 /*
  * This function was called in order to parse a CDROM device, but
  * .vmdk files are for harddisk devices only. Just ignore it,
@@ -2285,7 +2286,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
  * handle it.
  */
 goto ignore;
-} else if (virStringHasCaseSuffix(fileName, ".iso")) {
+} else if (fileName && virStringHasCaseSuffix(fileName, ".iso")) {
 char *tmp;
 
 if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) {
@@ -2306,7 +2307,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 } else if (deviceType && STRCASEEQ(deviceType, "atapi-cdrom")) {
 virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK);
 
-if (STRCASEEQ(fileName, "auto detect")) {
+if (fileName && STRCASEEQ(fileName, "auto detect")) {
 ignore_value(virDomainDiskSetSource(*def, NULL));
 (*def)->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_OPTIONAL;
 } else if (virDomainDiskSetSource(*def, fileName) < 0) {
@@ -2317,7 +2318,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 (*def)->device = VIR_DOMAIN_DISK_DEVICE_LUN;
 virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK);
 
-if (STRCASEEQ(fileName, "auto detect")) {
+if (fileName && STRCASEEQ(fileName, "auto detect")) {
 ignore_value(virDomainDiskSetSource(*def, NULL));
 (*def)->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_OPTIONAL;
 } else if (virDomainDiskSetSource(*def, fileName) < 0) {
@@ -2325,7 +2326,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 }
 } else if (busType == VIR_DOMAIN_DISK_BUS_SCSI &&
deviceType && STRCASEEQ(deviceType, "scsi-passthru")) {
-if (STRPREFIX(fileName, "/vmfs/devices/cdrom/")) {
+if (fileName && STRPREFIX(fileName, "/vmfs/devices/cdrom/")) {
 /* SCSI-passthru CD-ROMs actually are device='lun' */
 (*def)->device = VIR_DOMAIN_DISK_DEVICE_LUN;
 virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK);
@@ -2341,7 +2342,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
  */
 goto ign

[PATCH v2 0/2] vmx: make 'fileName' optional for CD-ROMs

2020-03-18 Thread Pino Toscano
v1 is:
https://www.redhat.com/archives/libvir-list/2020-March/msg00616.html

Changes from v1:
- added a patch to shortcut few checks
- use NULLSTR
- handle floppies (poor guys)

Pino Toscano (2):
  vmx: shortcut earlier few 'ignore' cases in virVMXParseDisk()
  vmx: make 'fileName' optional for CD-ROMs

 src/vmx/vmx.c | 67 ++-
 .../vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx |  4 ++
 .../vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml | 23 +++
 tests/vmx2xmltest.c   |  1 +
 4 files changed, 62 insertions(+), 33 deletions(-)
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml

-- 
2.25.1



[PATCH] vmx: make 'fileName' optional for CD-ROMs

2020-03-17 Thread Pino Toscano
It seems like CD-ROMs may have no 'fileName' property specified in case
there is nothing configured as attachment for the drive. Hence, make
sure that virVMXParseDisk() do not consider it mandatory anymore,
considering it an empty block cdrom device. Sadly virVMXParseDisk() is
used also to parse disk and floppies, so make sure that a NULL fileName
is handled in cdrom-related paths.

https://bugzilla.redhat.com/show_bug.cgi?id=1808610

Signed-off-by: Pino Toscano 
---
 src/vmx/vmx.c | 22 ++
 .../vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx |  4 
 .../vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml | 23 +++
 tests/vmx2xmltest.c   |  1 +
 4 files changed, 40 insertions(+), 10 deletions(-)
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index f0140129a2..58ae966fd4 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2207,7 +2207,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 goto cleanup;
 
 /* vmx:fileName -> def:src, def:type */
-if (virVMXGetConfigString(conf, fileName_name, , false) < 0)
+if (virVMXGetConfigString(conf, fileName_name, , true) < 0)
 goto cleanup;
 
 /* vmx:writeThrough -> def:cachemode */
@@ -2218,7 +2218,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 
 /* Setup virDomainDiskDef */
 if (device == VIR_DOMAIN_DISK_DEVICE_DISK) {
-if (virStringHasCaseSuffix(fileName, ".vmdk")) {
+if (fileName && virStringHasCaseSuffix(fileName, ".vmdk")) {
 char *tmp;
 
 if (deviceType != NULL) {
@@ -2254,7 +2254,8 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 if (mode)
 (*def)->transient = STRCASEEQ(mode,
   "independent-nonpersistent");
-} else if (virStringHasCaseSuffix(fileName, ".iso") ||
+} else if (fileName == NULL ||
+   virStringHasCaseSuffix(fileName, ".iso") ||
STREQ(fileName, "emptyBackingString") ||
(deviceType &&
 (STRCASEEQ(deviceType, "atapi-cdrom") ||
@@ -2277,7 +2278,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 goto cleanup;
 }
 } else if (device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
-if (virStringHasCaseSuffix(fileName, ".iso")) {
+if (fileName && virStringHasCaseSuffix(fileName, ".iso")) {
 char *tmp;
 
 if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) {
@@ -2295,7 +2296,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 goto cleanup;
 }
 VIR_FREE(tmp);
-} else if (virStringHasCaseSuffix(fileName, ".vmdk")) {
+} else if (fileName && virStringHasCaseSuffix(fileName, ".vmdk")) {
 /*
  * This function was called in order to parse a CDROM device, but
  * .vmdk files are for harddisk devices only. Just ignore it,
@@ -2306,7 +2307,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 } else if (deviceType && STRCASEEQ(deviceType, "atapi-cdrom")) {
 virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK);
 
-if (STRCASEEQ(fileName, "auto detect")) {
+if (fileName && STRCASEEQ(fileName, "auto detect")) {
 ignore_value(virDomainDiskSetSource(*def, NULL));
 (*def)->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_OPTIONAL;
 } else if (virDomainDiskSetSource(*def, fileName) < 0) {
@@ -2317,7 +2318,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 (*def)->device = VIR_DOMAIN_DISK_DEVICE_LUN;
 virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK);
 
-if (STRCASEEQ(fileName, "auto detect")) {
+if (fileName && STRCASEEQ(fileName, "auto detect")) {
 ignore_value(virDomainDiskSetSource(*def, NULL));
 (*def)->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_OPTIONAL;
 } else if (virDomainDiskSetSource(*def, fileName) < 0) {
@@ -2325,7 +2326,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr 
xmlopt, virConfPtr con
 }
 } else if (busType == VIR_DOMAIN_DISK_BUS_SCSI &&
deviceType && STRCASEEQ(deviceType, "scsi-

Re: [libvirt] [PATCH 09/23] util: introduce virFileDataSync

2020-01-06 Thread Pino Toscano
On Thursday, 2 January 2020 15:53:43 CET Daniel P. Berrangé wrote:
> A wrapper that calls g_fsync on Win32/macOS and fdatasync
> elsewhere. g_fsync is a stronger flush than we need but it
> satisfies the caller's requirements & matches the approach
> gnulib takes.
> 
> [...]
> +int
> +virFileDataSync(int fd)
> +{
> +#if defined(__APPLE__) || defined(WIN32)
> +return g_fsync(fd);
> +#else
> +return fdatasync(fd);
> +#endif

Why not just simply add a configure check for the fdatasync function?
This way there is no need to hardcode OSes/platforms.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 18/23] util: use realpath/g_canonicalize_filename

2020-01-06 Thread Pino Toscano
On Thursday, 2 January 2020 15:53:52 CET Daniel P. Berrangé wrote:
> The canonicalize_file_name(path) is equivalent to calling
> realpath(path, NULL). Passing NULL for the second arg of
> realpath is not standardized behaviour, however, Linux,
> FreeBSD > 6.4 and macOS > 10.5 all support this critical
> extension.

This is not correct: POSIX.1-2008 standardizes null as possible value
for the second argument; see:
https://pubs.opengroup.org/onlinepubs/9699919799/functions/realpath.html

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 06/11] esx: improve some of the virErrorNumber used

2019-12-20 Thread Pino Toscano
A lot of virReportError() calls use VIR_ERR_INTERNAL_ERROR to represent
the number of the error, even in cases where there is one fitting more.
Hence, replace some of them with better virErrorNumber values.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_network_driver.c|  4 ++--
 src/esx/esx_storage_backend_iscsi.c |  4 ++--
 src/esx/esx_storage_backend_vmfs.c  | 12 +-
 src/esx/esx_util.c  |  4 ++--
 src/esx/esx_vi.c| 36 ++---
 5 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index f14d309293..2a76831606 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -355,7 +355,7 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml)
 for (hostPortGroup = hostPortGroupList; hostPortGroup;
  hostPortGroup = hostPortGroup->_next) {
 if (STREQ(def->portGroups[i].name, hostPortGroup->spec->name)) 
{
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NETWORK_EXIST,
_("HostPortGroup with name '%s' exists 
already"),
def->portGroups[i].name);
 goto cleanup;
@@ -388,7 +388,7 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml)
 
 if (def->forward.ifs[i].type !=
 VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_SUPPORT,
_("unsupported device type in network %s "
  "interface pool"),
def->name);
diff --git a/src/esx/esx_storage_backend_iscsi.c 
b/src/esx/esx_storage_backend_iscsi.c
index cd83a0fbfd..1e54cef948 100644
--- a/src/esx/esx_storage_backend_iscsi.c
+++ b/src/esx/esx_storage_backend_iscsi.c
@@ -321,7 +321,7 @@ esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned 
int flags)
 
 if (!target) {
 /* pool not found */
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_STORAGE_POOL,
_("Could not find storage pool with name '%s'"),
pool->name);
 goto cleanup;
@@ -701,7 +701,7 @@ esxStorageVolGetXMLDesc(virStorageVolPtr volume,
 }
 
 if (!scsiLun) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_STORAGE_VOL,
_("Could find volume with name: %s"), volume->name);
 goto cleanup;
 }
diff --git a/src/esx/esx_storage_backend_vmfs.c 
b/src/esx/esx_storage_backend_vmfs.c
index 70005717cb..d4b23c6d51 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -906,7 +906,7 @@ esxStorageVolCreateXML(virStoragePoolPtr pool,
 goto cleanup;
 
 if (def->type != VIR_STORAGE_VOL_FILE) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+virReportError(VIR_ERR_NO_SUPPORT, "%s",
_("Creating non-file volumes is not supported"));
 goto cleanup;
 }
@@ -922,7 +922,7 @@ esxStorageVolCreateXML(virStoragePoolPtr pool,
 }
 
 if (!virStringHasCaseSuffix(def->name, ".vmdk")) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_SUPPORT,
_("Volume name '%s' has unsupported suffix, "
  "expecting '.vmdk'"), def->name);
 goto cleanup;
@@ -1041,7 +1041,7 @@ esxStorageVolCreateXML(virStoragePoolPtr pool,
 key = g_strdup(datastorePath);
 }
 } else {
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_SUPPORT,
_("Creation of %s volumes is not supported"),
virStorageFileFormatTypeToString(def->target.format));
 goto cleanup;
@@ -1120,7 +1120,7 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool,
 goto cleanup;
 
 if (def->type != VIR_STORAGE_VOL_FILE) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+virReportError(VIR_ERR_NO_SUPPORT, "%s",
_("Creating non-file volumes is not supported"));
 goto cleanup;
 }
@@ -1136,7 +1136,7 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool,
 }
 
 if (!virStringHasCaseSuffix(def->name, ".vmdk")) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_NO_SUPPORT,
_("Volume name '%s' has unsupported suffix, "
  "expecting '.vmdk'"), def->name);
 goto cleanup;
@@ -

[libvirt] [PATCH v3 02/11] esx: implement connectListAllNetworks

2019-12-20 Thread Pino Toscano
Implement the .connectListAllNetworks networks API in the esx network
driver.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_network_driver.c | 64 
 1 file changed, 64 insertions(+)

diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index 4f359c61e2..f14d309293 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -863,6 +863,69 @@ esxNetworkIsPersistent(virNetworkPtr network G_GNUC_UNUSED)
 
 
 
+#define MATCH(FLAG) (flags & (FLAG))
+static int
+esxConnectListAllNetworks(virConnectPtr conn,
+  virNetworkPtr **nets,
+  unsigned int flags)
+{
+int ret = -1;
+esxPrivate *priv = conn->privateData;
+esxVI_HostVirtualSwitch *hostVirtualSwitchList = NULL;
+esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL;
+size_t count = 0;
+size_t i;
+
+virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
+
+/*
+ * ESX networks are always active, persistent, and
+ * autostarted, so return zero elements in case we are asked
+ * for networks different than that.
+ */
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_ACTIVE) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_ACTIVE)))
+return 0;
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_PERSISTENT) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_PERSISTENT)))
+return 0;
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_AUTOSTART) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_AUTOSTART)))
+return 0;
+
+if (esxVI_EnsureSession(priv->primary) < 0 ||
+esxVI_LookupHostVirtualSwitchList(priv->primary,
+  ) < 0) {
+return -1;
+}
+
+for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch;
+ hostVirtualSwitch = hostVirtualSwitch->_next) {
+virNetworkPtr net = virtualswitchToNetwork(conn, hostVirtualSwitch);
+
+if (VIR_APPEND_ELEMENT(*nets, count, net) < 0)
+goto cleanup;
+}
+
+ret = count;
+
+ cleanup:
+if (ret < 0) {
+if (*nets) {
+for (i = 0; i < count; ++i)
+VIR_FREE((*nets)[i]);
+VIR_FREE(*nets);
+}
+}
+
+esxVI_HostVirtualSwitch_Free();
+
+return ret;
+}
+#undef MATCH
+
+
+
 virNetworkDriver esxNetworkDriver = {
 .connectNumOfNetworks = esxConnectNumOfNetworks, /* 0.10.0 */
 .connectListNetworks = esxConnectListNetworks, /* 0.10.0 */
@@ -877,4 +940,5 @@ virNetworkDriver esxNetworkDriver = {
 .networkSetAutostart = esxNetworkSetAutostart, /* 0.10.0 */
 .networkIsActive = esxNetworkIsActive, /* 0.10.0 */
 .networkIsPersistent = esxNetworkIsPersistent, /* 0.10.0 */
+.connectListAllNetworks = esxConnectListAllNetworks, /* 6.0.0 */
 };
-- 
2.24.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v3 07/11] esx: implement domainGetHostname

2019-12-20 Thread Pino Toscano
Implement the .domainGetHostname hypervisor driver API to get the
hostname of a running guest (needs VMware Tools).

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

diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in
index ac7bc645d1..465daafc2e 100644
--- a/docs/drvesx.html.in
+++ b/docs/drvesx.html.in
@@ -789,6 +789,9 @@ Enter administrator password for example-vcenter.com:
 performed the ESX server raises an error and the driver reports it.
 
 
+
+virDomainGetHostname
+
 
 virDomainReboot
 
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 0ede65279a..39e3faeb8f 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -5069,6 +5069,59 @@ esxDomainHasManagedSaveImage(virDomainPtr domain, 
unsigned int flags)
 }
 
 
+static char *
+esxDomainGetHostname(virDomainPtr domain,
+ unsigned int flags)
+{
+esxPrivate *priv = domain->conn->privateData;
+esxVI_String *propertyNameList = NULL;
+esxVI_ObjectContent *virtualMachine = NULL;
+esxVI_VirtualMachinePowerState powerState;
+char *hostname = NULL;
+char *new_hostname = NULL;
+
+virCheckFlags(0, NULL);
+
+if (esxVI_EnsureSession(priv->primary) < 0)
+return NULL;
+
+if (esxVI_String_AppendValueListToList(,
+   "runtime.powerState\0"
+   "guest.hostName") < 0 ||
+esxVI_LookupVirtualMachineByUuid(priv->primary, domain->uuid,
+ propertyNameList, ,
+ esxVI_Occurrence_OptionalItem) ||
+esxVI_GetVirtualMachinePowerState(virtualMachine, ) < 0) {
+goto cleanup;
+}
+
+if (powerState != esxVI_VirtualMachinePowerState_PoweredOn) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("Domain is not powered on"));
+goto cleanup;
+}
+
+if (esxVI_GetStringValue(virtualMachine, "guest.hostName",
+ , esxVI_Occurrence_OptionalItem) < 0) {
+goto cleanup;
+}
+
+if (!hostname) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("hostName field not available (missing VMware 
Tools?)"));
+goto cleanup;
+}
+
+new_hostname = g_strdup(hostname);
+
+ cleanup:
+esxVI_String_Free();
+esxVI_ObjectContent_Free();
+
+return new_hostname;
+}
+
+
 static virHypervisorDriver esxHypervisorDriver = {
 .name = "ESX",
 .connectOpen = esxConnectOpen, /* 0.7.0 */
@@ -5149,6 +5202,7 @@ static virHypervisorDriver esxHypervisorDriver = {
 .domainSnapshotDelete = esxDomainSnapshotDelete, /* 0.8.0 */
 .connectIsAlive = esxConnectIsAlive, /* 0.9.8 */
 .domainHasManagedSaveImage = esxDomainHasManagedSaveImage, /* 1.2.13 */
+.domainGetHostname = esxDomainGetHostname, /* 6.0.0 */
 };
 
 
-- 
2.24.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v3 08/11] esx: generator: fix free of elements in lists

2019-12-20 Thread Pino Toscano
When a list is freed, we iterate through all the items, invoking the
free function for each; the actual free function called for each element
is the function of the actual type of each element, and thus the @_next
pointer in the element struct has the same type as the element itself.
Currently, the free function gets the parent of the current element
type, and invoke its free function to continue freeing the list.
However, in case the hierarchy of the classes has more than 1 level
(i.e. Class <- SubClass <- SubSubClass), the invoked free function is
only the parent class' one, and not the actual base class of the
hierarchy.

To fix that, change the generator to get the ancestor of a class, and
invoking that instead.  Also, avoid to set the @_next back, as it is not
needed.

Fixes commits 5cff36e39ae691fbd7c40597df1732eecf294150 and
f76c6dde2e33233566e886d96e76b5fe0c102d9a.

Signed-off-by: Pino Toscano 
---
 scripts/esx_vi_generator.py | 26 +-
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/scripts/esx_vi_generator.py b/scripts/esx_vi_generator.py
index 048f5dde9e..9e3151943b 100755
--- a/scripts/esx_vi_generator.py
+++ b/scripts/esx_vi_generator.py
@@ -751,13 +751,13 @@ class Object(GenericObject):
 source += "{\n"
 
 if self.features & Object.FEATURE__LIST:
-if self.extends is not None:
+ancestor = get_ancestor(self)
+if ancestor:
 # avoid "dereferencing type-punned pointer will break
 # strict-aliasing rules" warnings
-source += "esxVI_%s *next = (esxVI_%s *)item->_next;\n\n" \
-  % (self.extends, self.extends)
-source += "esxVI_%s_Free();\n" % self.extends
-source += "item->_next = (esxVI_%s *)next;\n\n" % self.name
+source += "esxVI_%s *baseNext = (esxVI_%s 
*)item->_next;\n" \
+  % (ancestor, ancestor)
+source += "esxVI_%s_Free();\n\n" % ancestor
 else:
 source += "esxVI_%s_Free(>_next);\n\n" % self.name
 
@@ -1250,6 +1250,21 @@ def is_known_type(type):
 type in enums_by_name)
 
 
+def get_ancestor(obj):
+if not obj.extends:
+return None
+ancestor = None
+try:
+ancestor = ancestor_by_name[obj.extends]
+except KeyError:
+parent = objects_by_name[obj.extends]
+ancestor = get_ancestor(parent)
+if not ancestor:
+ancestor = parent.name
+ancestor_by_name[name] = ancestor
+return ancestor
+
+
 def open_and_print(filename):
 if filename.startswith("./"):
 print("  GEN  " + filename[2:])
@@ -1346,6 +1361,7 @@ managed_objects_by_name = {}
 enums_by_name = {}
 methods_by_name = {}
 block = None
+ancestor_by_name = {}
 
 
 # parse input file
-- 
2.24.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v3 10/11] esx: implement domainInterfaceAddresses

2019-12-20 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 465daafc2e..c4a2ae78a8 100644
--- a/docs/drvesx.html.in
+++ b/docs/drvesx.html.in
@@ -792,6 +792,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 39e3faeb8f..73b8f32f1b 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -5122,6 +5122,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,
+

[libvirt] [PATCH v3 04/11] esx: split scsilunToStorageVol helper

2019-12-20 Thread Pino Toscano
Move the creation of a virStorageVolPtr object from the esxVI_ScsiLun
object of a SCSI lun out of esxStorageVolLookupByName and
esxStorageVolLookupByPath in an own helper. This way it can be used
also in other functions.

Signed-off-by: Pino Toscano 
Reviewed-by: Cole Robinson 
---
 src/esx/esx_storage_backend_iscsi.c | 60 +++--
 1 file changed, 31 insertions(+), 29 deletions(-)

diff --git a/src/esx/esx_storage_backend_iscsi.c 
b/src/esx/esx_storage_backend_iscsi.c
index b6e0841dda..50de7d88ac 100644
--- a/src/esx/esx_storage_backend_iscsi.c
+++ b/src/esx/esx_storage_backend_iscsi.c
@@ -440,6 +440,35 @@ esxStoragePoolListVolumes(virStoragePoolPtr pool, char 
**const names,
 
 
 
+static virStorageVolPtr
+scsiLunToStorageVol(virConnectPtr conn, esxVI_ScsiLun *scsiLun,
+const char *pool)
+{
+/* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */
+unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5];
+char uuid_string[VIR_UUID_STRING_BUFLEN] = "";
+
+/*
+ * ScsiLun provides a UUID field that is unique across
+ * multiple servers. But this field length is ~55 characters
+ * compute MD5 hash to transform it to an acceptable
+ * libvirt format
+ */
+if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, scsiLun->uuid, md5) < 0)
+return NULL;
+virUUIDFormat(md5, uuid_string);
+
+/*
+ * ScsiLun provides displayName and canonicalName but both are
+ * optional and its observed that they can be NULL, using
+ * deviceName to create volume.
+ */
+return virGetStorageVol(conn, pool, scsiLun->deviceName, uuid_string,
+, NULL);
+}
+
+
+
 static virStorageVolPtr
 esxStorageVolLookupByName(virStoragePoolPtr pool,
   const char *name)
@@ -448,9 +477,6 @@ esxStorageVolLookupByName(virStoragePoolPtr pool,
 esxPrivate *priv = pool->conn->privateData;
 esxVI_ScsiLun *scsiLunList = NULL;
 esxVI_ScsiLun *scsiLun;
-/* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */
-unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5];
-char uuid_string[VIR_UUID_STRING_BUFLEN] = "";
 
 if (esxVI_LookupScsiLunList(priv->primary, ) < 0)
 goto cleanup;
@@ -458,23 +484,7 @@ esxStorageVolLookupByName(virStoragePoolPtr pool,
 for (scsiLun = scsiLunList; scsiLun;
  scsiLun = scsiLun->_next) {
 if (STREQ(scsiLun->deviceName, name)) {
-/*
- * ScsiLun provides a UUID field that is unique across
- * multiple servers. But this field length is ~55 characters
- * compute MD5 hash to transform it to an acceptable
- * libvirt format
- */
-if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, scsiLun->uuid, md5) < 0)
-goto cleanup;
-virUUIDFormat(md5, uuid_string);
-
-/*
- * ScsiLun provides displayName and canonicalName but both are
- * optional and its observed that they can be NULL, using
- * deviceName to create volume.
- */
-volume = virGetStorageVol(pool->conn, pool->name, name, 
uuid_string,
-  , NULL);
+volume = scsiLunToStorageVol(pool->conn, scsiLun, pool->name);
 break;
 }
 }
@@ -496,9 +506,6 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char 
*path)
 esxVI_ScsiLun *scsiLun;
 esxVI_HostScsiDisk *hostScsiDisk = NULL;
 char *poolName = NULL;
-/* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */
-unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5];
-char uuid_string[VIR_UUID_STRING_BUFLEN] = "";
 
 if (esxVI_LookupScsiLunList(priv->primary, ) < 0)
 goto cleanup;
@@ -516,12 +523,7 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char 
*path)
 goto cleanup;
 }
 
-if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, scsiLun->uuid, md5) < 0)
-goto cleanup;
-virUUIDFormat(md5, uuid_string);
-
-volume = virGetStorageVol(conn, poolName, path, uuid_string,
-  , NULL);
+volume = scsiLunToStorageVol(conn, scsiLun, poolName);
 break;
 }
 }
-- 
2.24.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v3 05/11] esx: implement storagePoolListAllVolumes

2019-12-20 Thread Pino Toscano
Implement the .storagePoolListAllVolumes storage API in the esx storage
driver, and in all its subdrivers.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_storage_backend_iscsi.c | 68 +++
 src/esx/esx_storage_backend_vmfs.c  | 83 +
 src/esx/esx_storage_driver.c| 19 +++
 3 files changed, 170 insertions(+)

diff --git a/src/esx/esx_storage_backend_iscsi.c 
b/src/esx/esx_storage_backend_iscsi.c
index 50de7d88ac..cd83a0fbfd 100644
--- a/src/esx/esx_storage_backend_iscsi.c
+++ b/src/esx/esx_storage_backend_iscsi.c
@@ -850,6 +850,73 @@ esxConnectListAllStoragePools(virConnectPtr conn,
 
 
 
+static int
+esxStoragePoolListAllVolumes(virStoragePoolPtr pool,
+ virStorageVolPtr **vols,
+ unsigned int flags)
+{
+int ret = -1;
+size_t count = 0;
+esxPrivate *priv = pool->conn->privateData;
+esxVI_HostScsiTopologyLun *hostScsiTopologyLunList = NULL;
+esxVI_HostScsiTopologyLun *hostScsiTopologyLun;
+esxVI_ScsiLun *scsiLunList = NULL;
+esxVI_ScsiLun *scsiLun = NULL;
+size_t i;
+
+virCheckFlags(0, -1);
+
+if (esxVI_LookupHostScsiTopologyLunListByTargetName
+  (priv->primary, pool->name, ) < 0) {
+goto cleanup;
+}
+
+if (!hostScsiTopologyLunList) {
+/* iSCSI adapter may not be enabled on ESX host */
+return 0;
+}
+
+if (esxVI_LookupScsiLunList(priv->primary, ) < 0)
+goto cleanup;
+
+for (scsiLun = scsiLunList; scsiLun;
+ scsiLun = scsiLun->_next) {
+for (hostScsiTopologyLun = hostScsiTopologyLunList;
+ hostScsiTopologyLun;
+ hostScsiTopologyLun = hostScsiTopologyLun->_next) {
+if (STREQ(hostScsiTopologyLun->scsiLun, scsiLun->key)) {
+virStorageVolPtr volume;
+
+volume = scsiLunToStorageVol(pool->conn, scsiLun, pool->name);
+if (!volume)
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(*vols, count, volume) < 0)
+goto cleanup;
+}
+
+}
+}
+
+ret = count;
+
+ cleanup:
+if (ret < 0) {
+if (*vols) {
+for (i = 0; i < count; ++i)
+VIR_FREE((*vols)[i]);
+VIR_FREE(*vols);
+}
+}
+
+esxVI_HostScsiTopologyLun_Free();
+esxVI_ScsiLun_Free();
+
+return ret;
+}
+
+
+
 virStorageDriver esxStorageBackendISCSI = {
 .connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 1.0.1 */
 .connectListStoragePools = esxConnectListStoragePools, /* 1.0.1 */
@@ -871,4 +938,5 @@ virStorageDriver esxStorageBackendISCSI = {
 .storageVolWipe = esxStorageVolWipe, /* 1.0.1 */
 .storageVolGetPath = esxStorageVolGetPath, /* 1.0.1 */
 .connectListAllStoragePools = esxConnectListAllStoragePools, /* 6.0.0 */
+.storagePoolListAllVolumes = esxStoragePoolListAllVolumes, /* 6.0.0 */
 };
diff --git a/src/esx/esx_storage_backend_vmfs.c 
b/src/esx/esx_storage_backend_vmfs.c
index 1d7f9afda1..70005717cb 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -1565,6 +1565,88 @@ esxConnectListAllStoragePools(virConnectPtr conn,
 
 
 
+static int
+esxStoragePoolListAllVolumes(virStoragePoolPtr pool,
+ virStorageVolPtr **vols,
+ unsigned int flags)
+{
+int ret = -1;
+esxPrivate *priv = pool->conn->privateData;
+esxVI_HostDatastoreBrowserSearchResults *searchResultsList = NULL;
+esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL;
+esxVI_FileInfo *fileInfo = NULL;
+char *directoryAndFileName = NULL;
+size_t length;
+size_t count = 0;
+size_t i;
+
+virCheckFlags(0, -1);
+
+if (esxVI_LookupDatastoreContentByDatastoreName(priv->primary, pool->name,
+) < 0) {
+goto cleanup;
+}
+
+/* Interpret search result */
+for (searchResults = searchResultsList; searchResults;
+ searchResults = searchResults->_next) {
+VIR_FREE(directoryAndFileName);
+
+if (esxUtil_ParseDatastorePath(searchResults->folderPath, NULL, NULL,
+   ) < 0) {
+goto cleanup;
+}
+
+/* Strip trailing separators */
+length = strlen(directoryAndFileName);
+
+while (length > 0 && directoryAndFileName[length - 1] == '/') {
+directoryAndFileName[length - 1] = '\0';
+--length;
+}
+
+/* Build volume names */
+for (fileInfo = searchResults->file; fileInfo;
+ fileInfo = fileInfo->_next) {
+g_autofree char *datastorePath = NULL;
+virStorageVolPtr volume;
+
+if (length < 1) {
+datastorePath = g_strdup(fi

[libvirt] [PATCH v3 00/11] esx: various improvements

2019-12-20 Thread Pino Toscano
- fix a bug in the esx VI generator
- implement connectListAllStoragePools, so
  virConnectListAllStoragePools() works
- implement connectListAllNetworks, so virConnectListAllNetworks()
  works
- implement storagePoolListAllVolumes, so virStoragePoolListAllVolumes()
  works
- implement domainGetHostname, so virDomainGetHostname() works
- implement domainInterfaceAddresses only for
  VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT, so
  virDomainInterfaceAddresses() partially works
- improve virErrorNumber for some virReportError() calls

TODO:
- handle the comments in v2 about esxStoragePoolListAllVolumes()

Changes from v2:
- pushed patches reviewed with no changes required
- integrated the Reviewed-By in patches that required very minor
  changes, will push them on request
- fixed bool/size_t error/count handling
- bumped API version numbers to 6.0.0
- fixed a bug in the esx VI generator
- implemented domainGetHostname
- implemented domainInterfaceAddresses


Pino Toscano (11):
  esx: implement connectListAllStoragePools
  esx: implement connectListAllNetworks
  esx: split datastorePathToStorageVol helper
  esx: split scsilunToStorageVol helper
  esx: implement storagePoolListAllVolumes
  esx: improve some of the virErrorNumber used
  esx: implement domainGetHostname
  esx: generator: fix free of elements in lists
  esx: generator: add GuestNicInfo object
  esx: implement domainInterfaceAddresses
  docs: document implemented APIs in esx

 docs/drvesx.html.in |   7 +
 docs/news.xml   |  14 ++
 scripts/esx_vi_generator.py |  27 +++-
 src/esx/esx_driver.c| 220 
 src/esx/esx_network_driver.c|  68 -
 src/esx/esx_storage_backend_iscsi.c | 202 +
 src/esx/esx_storage_backend_vmfs.c  | 220 ++--
 src/esx/esx_storage_driver.c|  84 +++
 src/esx/esx_util.c  |   4 +-
 src/esx/esx_vi.c|  36 ++---
 src/esx/esx_vi_generator.input  |  54 +++
 11 files changed, 862 insertions(+), 74 deletions(-)

-- 
2.24.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v3 11/11] docs: document implemented APIs in esx

2019-12-20 Thread Pino Toscano
Signed-off-by: Pino Toscano 
---
 docs/news.xml | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 055353b9a5..6c29011d53 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -86,6 +86,20 @@
   lzop should be used.
 
   
+  
+
+  esx: implement various APIs
+
+
+  The virConnectListAllStoragePools(),
+  virConnectListAllNetworks(),
+  virStoragePoolListAllVolumes(),
+  virDomainGetHostname,
+  and virDomainInterfaceAddresses (only for
+  VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT source)
+  APIs were implemented in the esx driver.
+
+  
 
 
 
-- 
2.24.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v3 09/11] esx: generator: add GuestNicInfo object

2019-12-20 Thread Pino Toscano
Add the definition of the GuestNicInfo object, with all the required
objects for it.

Signed-off-by: Pino Toscano 
---
 scripts/esx_vi_generator.py|  1 +
 src/esx/esx_vi_generator.input | 54 ++
 2 files changed, 55 insertions(+)

diff --git a/scripts/esx_vi_generator.py b/scripts/esx_vi_generator.py
index 9e3151943b..761bb2d8de 100755
--- a/scripts/esx_vi_generator.py
+++ b/scripts/esx_vi_generator.py
@@ -1297,6 +1297,7 @@ additional_object_features = {
 "DatastoreHostMount": (Object.FEATURE__DEEP_COPY | Object.FEATURE__LIST |
Object.FEATURE__ANY_TYPE),
 "DatastoreInfo": Object.FEATURE__ANY_TYPE | Object.FEATURE__DYNAMIC_CAST,
+"GuestNicInfo": Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE,
 "HostConfigManager": Object.FEATURE__ANY_TYPE,
 "HostCpuIdInfo": Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE,
 "HostDatastoreBrowserSearchResults": (Object.FEATURE__LIST |
diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input
index 22c114e0aa..bd6ac72a18 100644
--- a/src/esx/esx_vi_generator.input
+++ b/src/esx/esx_vi_generator.input
@@ -277,6 +277,18 @@ object FolderFileQuery   extends FileQuery
 end
 
 
+object GuestNicInfo
+Boolean  connected  r
+Int  deviceConfigId r
+NetDnsConfigInfo dnsConfig  o
+String   ipAddress  ol
+NetIpConfigInfo  ipConfig   o
+String   macAddress o
+NetBIOSConfigInfonetBIOSConfig  o
+String   networko
+end
+
+
 object HostAutoStartManagerConfig
 AutoStartDefaultsdefaults   o
 AutoStartPowerInfo   powerInfo  ol
@@ -770,6 +782,48 @@ object NasDatastoreInfo  extends DatastoreInfo
 end
 
 
+object NetBIOSConfigInfo
+String   mode   r
+end
+
+
+object NetDhcpConfigInfo
+NetDhcpConfigInfoDhcpOptions ipv4   o
+NetDhcpConfigInfoDhcpOptions ipv6   o
+end
+
+
+object NetDhcpConfigInfoDhcpOptions
+KeyAnyValue  config ol
+Boolean  enable r
+end
+
+
+object NetDnsConfigInfo
+Boolean  dhcp   r
+String   domainName r
+String   hostName   r
+String   ipAddress  ol
+String   searchDomain   ol
+end
+
+
+object NetIpConfigInfo
+Boolean  autoConfigurationEnabled   o
+NetDhcpConfigInfodhcp   o
+NetIpConfigInfoIpAddress ipAddress  ol
+end
+
+
+object NetIpConfigInfoIpAddress
+String   ipAddress  r
+DateTime lifetime   o
+String   origin o
+Int  prefixLength   r
+String   state  o
+end
+
+
 object ObjectContent
 ManagedObjectReference   objr
 DynamicProperty  propSetol
-- 
2.24.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v3 01/11] esx: implement connectListAllStoragePools

2019-12-20 Thread Pino Toscano
Implement the .connectListAllStoragePools storage API in the esx storage
driver, and in all its subdrivers.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_storage_backend_iscsi.c | 70 +
 src/esx/esx_storage_backend_vmfs.c  | 96 +
 src/esx/esx_storage_driver.c| 65 +++
 3 files changed, 231 insertions(+)

diff --git a/src/esx/esx_storage_backend_iscsi.c 
b/src/esx/esx_storage_backend_iscsi.c
index 72ab0d3cb0..b6e0841dda 100644
--- a/src/esx/esx_storage_backend_iscsi.c
+++ b/src/esx/esx_storage_backend_iscsi.c
@@ -779,6 +779,75 @@ esxStorageVolGetPath(virStorageVolPtr volume)
 
 
 
+#define MATCH(FLAG) (flags & (FLAG))
+static int
+esxConnectListAllStoragePools(virConnectPtr conn,
+  virStoragePoolPtr **pools,
+  unsigned int flags)
+{
+int ret = -1;
+size_t count = 0;
+esxPrivate *priv = conn->privateData;
+esxVI_HostInternetScsiHba *hostInternetScsiHba = NULL;
+esxVI_HostInternetScsiHbaStaticTarget *target;
+size_t i;
+
+/* this driver provides only iSCSI pools */
+if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE) &&
+!(MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_ISCSI)))
+return 0;
+
+if (esxVI_LookupHostInternetScsiHba(priv->primary,
+) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Unable to obtain iSCSI adapter"));
+goto cleanup;
+}
+
+/* FIXME: code looks for software iSCSI adapter only */
+if (!hostInternetScsiHba) {
+/* iSCSI adapter may not be enabled for this host */
+return 0;
+}
+
+/*
+ * ESX has two kind of targets:
+ * 1. staticIscsiTargets
+ * 2. dynamicIscsiTargets
+ * For each dynamic target if it's reachable a static target is added.
+ * return iSCSI names for all static targets to avoid duplicate names.
+ */
+for (target = hostInternetScsiHba->configuredStaticTarget;
+ target; target = target->_next) {
+virStoragePoolPtr pool;
+
+pool = targetToStoragePool(conn, target->iScsiName, target);
+if (!pool)
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(*pools, count, pool) < 0)
+goto cleanup;
+}
+
+ret = count;
+
+ cleanup:
+if (ret < 0) {
+if (*pools) {
+for (i = 0; i < count; ++i)
+VIR_FREE((*pools)[i]);
+VIR_FREE(*pools);
+}
+}
+
+esxVI_HostInternetScsiHba_Free();
+
+return ret;
+}
+#undef MATCH
+
+
+
 virStorageDriver esxStorageBackendISCSI = {
 .connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 1.0.1 */
 .connectListStoragePools = esxConnectListStoragePools, /* 1.0.1 */
@@ -799,4 +868,5 @@ virStorageDriver esxStorageBackendISCSI = {
 .storageVolDelete = esxStorageVolDelete, /* 1.0.1 */
 .storageVolWipe = esxStorageVolWipe, /* 1.0.1 */
 .storageVolGetPath = esxStorageVolGetPath, /* 1.0.1 */
+.connectListAllStoragePools = esxConnectListAllStoragePools, /* 6.0.0 */
 };
diff --git a/src/esx/esx_storage_backend_vmfs.c 
b/src/esx/esx_storage_backend_vmfs.c
index 61b30c3c1d..41f0d4577b 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -1461,6 +1461,101 @@ esxStorageVolGetPath(virStorageVolPtr volume)
 
 
 
+#define MATCH(FLAG) (flags & (FLAG))
+static int
+esxConnectListAllStoragePools(virConnectPtr conn,
+  virStoragePoolPtr **pools,
+  unsigned int flags)
+{
+int ret = -1;
+esxPrivate *priv = conn->privateData;
+esxVI_String *propertyNameList = NULL;
+esxVI_DynamicProperty *dynamicProperty = NULL;
+esxVI_ObjectContent *datastoreList = NULL;
+esxVI_ObjectContent *datastore = NULL;
+size_t count = 0;
+size_t i;
+virStoragePoolPtr pool;
+const bool checkPoolType = 
MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE);
+
+if (esxVI_String_AppendValueToList(,
+   "summary.name") < 0) {
+goto cleanup;
+}
+
+if (checkPoolType &&
+esxVI_String_AppendValueToList(,
+   "info") < 0) {
+goto cleanup;
+}
+
+if (esxVI_LookupDatastoreList(priv->primary, propertyNameList,
+  ) < 0) {
+goto cleanup;
+}
+
+for (datastore = datastoreList; datastore;
+ datastore = datastore->_next) {
+const char *name = NULL;
+
+for (dynamicProperty = datastore->propSet; dynamicProperty;
+ dynamicProperty = dynamicProperty->_next) {
+if (STREQ(dynamicProperty->name, "summary.name")) {
+if (esxVI_AnyType_ExpectType

[libvirt] [PATCH v3 03/11] esx: split datastorePathToStorageVol helper

2019-12-20 Thread Pino Toscano
Move the creation of a virStorageVolPtr object by lookup out of
esxStorageVolLookupByPath in an own helper. This way it can be used
also in other functions.

Signed-off-by: Pino Toscano 
Reviewed-by: Cole Robinson 
---
 src/esx/esx_storage_backend_vmfs.c | 29 +++--
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/src/esx/esx_storage_backend_vmfs.c 
b/src/esx/esx_storage_backend_vmfs.c
index 41f0d4577b..1d7f9afda1 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -695,32 +695,41 @@ esxStorageVolLookupByName(virStoragePoolPtr pool,
 
 
 
+static virStorageVolPtr
+datastorePathToStorageVol(virConnectPtr conn, const char *pool,
+  const char *path)
+{
+esxPrivate *priv = conn->privateData;
+g_autofree char *key = NULL;
+
+if (esxVI_LookupStorageVolumeKeyByDatastorePath(priv->primary, path,
+) < 0) {
+return NULL;
+}
+
+return virGetStorageVol(conn, pool, path, key,
+, NULL);
+}
+
+
+
 static virStorageVolPtr
 esxStorageVolLookupByPath(virConnectPtr conn, const char *path)
 {
 virStorageVolPtr volume = NULL;
-esxPrivate *priv = conn->privateData;
 char *datastoreName = NULL;
 char *directoryAndFileName = NULL;
-char *key = NULL;
 
 if (esxUtil_ParseDatastorePath(path, , NULL,
) < 0) {
 goto cleanup;
 }
 
-if (esxVI_LookupStorageVolumeKeyByDatastorePath(priv->primary, path,
-) < 0) {
-goto cleanup;
-}
-
-volume = virGetStorageVol(conn, datastoreName, directoryAndFileName, key,
-  , NULL);
+volume = datastorePathToStorageVol(conn, datastoreName, 
directoryAndFileName);
 
  cleanup:
 VIR_FREE(datastoreName);
 VIR_FREE(directoryAndFileName);
-VIR_FREE(key);
 
 return volume;
 }
-- 
2.24.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [jenkins-ci PATCH 1/5] guests: Add mapping for augeas-lenses

2019-12-12 Thread Pino Toscano
On Thursday, 12 December 2019 16:26:26 CET Andrea Bolognani wrote:
> On Thu, 2019-12-12 at 16:05 +0100, Pino Toscano wrote:
> > On Thursday, 12 December 2019 15:31:04 CET Andrea Bolognani wrote:
> > > +  augeas-lenses:
> > > +default: augeas
> > > +deb: augeas-lenses
> > > +OpenSUSE: augeas-lenses
> > 
> > Why is this needed? The lenses must be already a dependency of the
> > augeas shared library, or at least of the tools.
> 
> Not on openSUSE, apparently:
> 
>   $ docker run --rm -it opensuse/leap:15.1
>   edb149088975:/ # zypper install augeas
>   Building repository 'Non-OSS Repository' cache ..
>   Building repository 'Main Repository' cache .
>   Building repository 'Main Update Repository' cache ..
>   Building repository 'Update Repository (Non-Oss)' cache .
>   Loading repository data...
>   Reading installed packages...
>   Resolving package dependencies...
> 
>   The following NEW package is going to be installed:
> augeas
> 
>   1 new package to install.
>   Overall download size: 130.1 KiB. Already cached: 0 B. After the operation, 
> additional 205.1 KiB will be used.
>   Continue? [y/n/v/...? shows all options] (y): y
>   Retrieving package augeas-1.10.1-lp151.2.3.x86_64 (1/1), 130.1 KiB 
> (205.1 KiB unpacked)
>   Retrieving: augeas-1.10.1-lp151.2.3.x86_64.rpm ..
> 
>   Checking for file conflicts: 
>   (1/1) Installing: augeas-1.10.1-lp151.2.3.x86_64 
>   edb149088975:/ # rpm -qa | grep augeas-lenses
>   edb149088975:/ # ls /usr/share/augeas/lenses
>   ls: cannot access '/usr/share/augeas/lenses': No such file or directory
>   edb149088975:/ #

Sigh...

OTOH, in the mapping file I see:

   augeas:
 default: augeas
 deb: augeas-tools

This means that most probably there are different packages installed
depending on the distro:
- on Debian-based distros you get the command line tools
- on some distros (mostly RPM-based) you get only the shared library
- on some other distros (FreeBSD, and Linux distros not handled yet)
  you may get either everything, or the command line tools

OTOH², looking at the spec [1] makes me sad, as nothing really
depends on augeas-lenses, neither the shared library package nor the
tools package :-( This is totally nonsense, you install augeas in
openSUSE and it is broken by default if you have recommends disabled...

So my suggestion is to limit the manual lenses installation only on
openSUSE, as any other distro is already doing a sane job in this case.

[1] 
https://build.opensuse.org/package/view_file/openSUSE:Factory/augeas/augeas.spec

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [jenkins-ci PATCH 1/5] guests: Add mapping for augeas-lenses

2019-12-12 Thread Pino Toscano
On Thursday, 12 December 2019 15:31:04 CET Andrea Bolognani wrote:
> Some distributions package augeas and the default lenses separately,
> so we need a mapping for the latter.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  guests/vars/mappings.yml | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml
> index d32ecf3..02bedab 100644
> --- a/guests/vars/mappings.yml
> +++ b/guests/vars/mappings.yml
> @@ -77,6 +77,11 @@ mappings:
>  default: augeas
>  deb: augeas-tools
>  
> +  augeas-lenses:
> +default: augeas
> +deb: augeas-lenses
> +OpenSUSE: augeas-lenses

Why is this needed? The lenses must be already a dependency of the
augeas shared library, or at least of the tools.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] virsh: limit completion of 'domhostname' to active domains

2019-11-20 Thread Pino Toscano
Getting the hostname of guest usually requires a in-guest agent, or
generally can be determined only on active domains.

Signed-off-by: Pino Toscano 
---
 tools/virsh-domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b248a15c16..6be9780836 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11668,7 +11668,7 @@ static const vshCmdInfo info_domhostname[] = {
 };
 
 static const vshCmdOptDef opts_domhostname[] = {
-VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
 {.name = NULL}
 };
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v2 04/12] esx: implement connectListAllStoragePools

2019-11-15 Thread Pino Toscano
Implement the .connectListAllStoragePools storage API in the esx storage
driver, and in all its subdrivers.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_storage_backend_iscsi.c | 72 +
 src/esx/esx_storage_backend_vmfs.c  | 98 +
 src/esx/esx_storage_driver.c| 68 
 3 files changed, 238 insertions(+)

diff --git a/src/esx/esx_storage_backend_iscsi.c 
b/src/esx/esx_storage_backend_iscsi.c
index 72ab0d3cb0..4f5d8e5e24 100644
--- a/src/esx/esx_storage_backend_iscsi.c
+++ b/src/esx/esx_storage_backend_iscsi.c
@@ -779,6 +779,77 @@ esxStorageVolGetPath(virStorageVolPtr volume)
 
 
 
+#define MATCH(FLAG) (flags & (FLAG))
+static int
+esxConnectListAllStoragePools(virConnectPtr conn,
+  virStoragePoolPtr **pools,
+  unsigned int flags)
+{
+bool success = false;
+size_t count = 0;
+esxPrivate *priv = conn->privateData;
+esxVI_HostInternetScsiHba *hostInternetScsiHba = NULL;
+esxVI_HostInternetScsiHbaStaticTarget *target;
+size_t i;
+
+/* this driver provides only iSCSI pools */
+if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE) &&
+!(MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_ISCSI)))
+return 0;
+
+if (esxVI_LookupHostInternetScsiHba(priv->primary,
+) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Unable to obtain iSCSI adapter"));
+goto cleanup;
+}
+
+/* FIXME: code looks for software iSCSI adapter only */
+if (!hostInternetScsiHba) {
+/* iSCSI adapter may not be enabled for this host */
+return 0;
+}
+
+/*
+ * ESX has two kind of targets:
+ * 1. staticIscsiTargets
+ * 2. dynamicIscsiTargets
+ * For each dynamic target if its reachable a static target is added.
+ * return iSCSI names for all static targets to avoid duplicate names.
+ */
+for (target = hostInternetScsiHba->configuredStaticTarget;
+ target; target = target->_next) {
+virStoragePoolPtr pool;
+
+pool = targetToStoragePool(conn, target->iScsiName, target);
+if (!pool)
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(*pools, count, pool) < 0)
+goto cleanup;
+}
+
+success = true;
+
+ cleanup:
+if (! success) {
+if (*pools) {
+for (i = 0; i < count; ++i)
+VIR_FREE((*pools)[i]);
+VIR_FREE(*pools);
+}
+
+count = -1;
+}
+
+esxVI_HostInternetScsiHba_Free();
+
+return count;
+}
+#undef MATCH
+
+
+
 virStorageDriver esxStorageBackendISCSI = {
 .connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 1.0.1 */
 .connectListStoragePools = esxConnectListStoragePools, /* 1.0.1 */
@@ -799,4 +870,5 @@ virStorageDriver esxStorageBackendISCSI = {
 .storageVolDelete = esxStorageVolDelete, /* 1.0.1 */
 .storageVolWipe = esxStorageVolWipe, /* 1.0.1 */
 .storageVolGetPath = esxStorageVolGetPath, /* 1.0.1 */
+.connectListAllStoragePools = esxConnectListAllStoragePools, /* 5.10.0 */
 };
diff --git a/src/esx/esx_storage_backend_vmfs.c 
b/src/esx/esx_storage_backend_vmfs.c
index b890825a40..05b273aed7 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -1460,6 +1460,103 @@ esxStorageVolGetPath(virStorageVolPtr volume)
 
 
 
+#define MATCH(FLAG) (flags & (FLAG))
+static int
+esxConnectListAllStoragePools(virConnectPtr conn,
+  virStoragePoolPtr **pools,
+  unsigned int flags)
+{
+bool success = false;
+esxPrivate *priv = conn->privateData;
+esxVI_String *propertyNameList = NULL;
+esxVI_DynamicProperty *dynamicProperty = NULL;
+esxVI_ObjectContent *datastoreList = NULL;
+esxVI_ObjectContent *datastore = NULL;
+size_t count = 0;
+size_t i;
+virStoragePoolPtr pool;
+const bool checkPoolType = 
MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE);
+
+if (esxVI_String_AppendValueToList(,
+   "summary.name") < 0) {
+goto cleanup;
+}
+
+if (checkPoolType &&
+esxVI_String_AppendValueToList(,
+   "info") < 0) {
+goto cleanup;
+}
+
+if (esxVI_LookupDatastoreList(priv->primary, propertyNameList,
+  ) < 0) {
+goto cleanup;
+}
+
+for (datastore = datastoreList; datastore;
+ datastore = datastore->_next) {
+const char *name = NULL;
+
+for (dynamicProperty = datastore->propSet; dynamicProperty;
+ dynamicProperty = dynamicProperty->_next) {
+if (STREQ(dynamicProperty->name, "summary.name")) {
+  

[libvirt] [PATCH v2 12/12] docs: document implemented APIs in esx

2019-11-15 Thread Pino Toscano
Signed-off-by: Pino Toscano 
---
 docs/news.xml | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index c362bf3a15..1718a686ce 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -55,6 +55,17 @@
   
 
 
+  
+
+  esx: implement various APIs
+
+
+  The virConnectListAllStoragePools(),
+  virConnectListAllNetworks() and
+  virStoragePoolListAllVolumes() APIs were implemented
+  in the esx driver.
+
+  
 
 
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v2 05/12] esx: split virtualswitchToNetwork helper

2019-11-15 Thread Pino Toscano
Move the creation of a virNetworkPtr object from the
esxVI_HostVirtualSwitch object of a virtual switch out of
esxNetworkLookupByName in an own helper. This way it can be used also
in other functions.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_network_driver.c | 34 ++
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index b5dcfe0a80..4f359c61e2 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -167,20 +167,11 @@ esxNetworkLookupByUUID(virConnectPtr conn, const unsigned 
char *uuid)
 
 
 static virNetworkPtr
-esxNetworkLookupByName(virConnectPtr conn, const char *name)
+virtualswitchToNetwork(virConnectPtr conn,
+   esxVI_HostVirtualSwitch *hostVirtualSwitch)
 {
-virNetworkPtr network = NULL;
-esxPrivate *priv = conn->privateData;
-esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL;
 unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; /* VIR_CRYPTO_HASH_SIZE_MD5 = 
VIR_UUID_BUFLEN = 16 */
 
-if (esxVI_EnsureSession(priv->primary) < 0 ||
-esxVI_LookupHostVirtualSwitchByName(priv->primary, name,
-,
-esxVI_Occurrence_RequiredItem) < 
0) {
-return NULL;
-}
-
 /*
  * HostVirtualSwitch doesn't have a UUID, but we can use the key property
  * as source for a UUID. The key is unique per host and cannot change
@@ -192,7 +183,26 @@ esxNetworkLookupByName(virConnectPtr conn, const char 
*name)
 if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, hostVirtualSwitch->key, md5) < 0)
 return NULL;
 
-network = virGetNetwork(conn, hostVirtualSwitch->name, md5);
+return virGetNetwork(conn, hostVirtualSwitch->name, md5);
+}
+
+
+
+static virNetworkPtr
+esxNetworkLookupByName(virConnectPtr conn, const char *name)
+{
+virNetworkPtr network = NULL;
+esxPrivate *priv = conn->privateData;
+esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL;
+
+if (esxVI_EnsureSession(priv->primary) < 0 ||
+esxVI_LookupHostVirtualSwitchByName(priv->primary, name,
+,
+esxVI_Occurrence_RequiredItem) < 
0) {
+return NULL;
+}
+
+network = virtualswitchToNetwork(conn, hostVirtualSwitch);
 
 esxVI_HostVirtualSwitch_Free();
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v2 06/12] esx: implement connectListAllNetworks

2019-11-15 Thread Pino Toscano
Implement the .connectListAllNetworks networks API in the esx network
driver.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_network_driver.c | 66 
 1 file changed, 66 insertions(+)

diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index 4f359c61e2..79a7ecae5f 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -863,6 +863,71 @@ esxNetworkIsPersistent(virNetworkPtr network G_GNUC_UNUSED)
 
 
 
+#define MATCH(FLAG) (flags & (FLAG))
+static int
+esxConnectListAllNetworks(virConnectPtr conn,
+  virNetworkPtr **nets,
+  unsigned int flags)
+{
+bool success = false;
+esxPrivate *priv = conn->privateData;
+esxVI_HostVirtualSwitch *hostVirtualSwitchList = NULL;
+esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL;
+size_t count = 0;
+size_t i;
+
+virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
+
+/*
+ * ESX networks are always active, persistent, and
+ * autostarted, so return zero elements in case we are asked
+ * for networks different than that.
+ */
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_ACTIVE) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_ACTIVE)))
+return 0;
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_PERSISTENT) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_PERSISTENT)))
+return 0;
+if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_AUTOSTART) &&
+!(MATCH(VIR_CONNECT_LIST_NETWORKS_AUTOSTART)))
+return 0;
+
+if (esxVI_EnsureSession(priv->primary) < 0 ||
+esxVI_LookupHostVirtualSwitchList(priv->primary,
+  ) < 0) {
+return -1;
+}
+
+for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch;
+ hostVirtualSwitch = hostVirtualSwitch->_next) {
+virNetworkPtr net = virtualswitchToNetwork(conn, hostVirtualSwitch);
+
+if (VIR_APPEND_ELEMENT(*nets, count, net) < 0)
+goto cleanup;
+}
+
+success = true;
+
+ cleanup:
+if (! success) {
+if (*nets) {
+for (i = 0; i < count; ++i)
+VIR_FREE((*nets)[i]);
+VIR_FREE(*nets);
+}
+
+count = -1;
+}
+
+esxVI_HostVirtualSwitch_Free();
+
+return count;
+}
+#undef MATCH
+
+
+
 virNetworkDriver esxNetworkDriver = {
 .connectNumOfNetworks = esxConnectNumOfNetworks, /* 0.10.0 */
 .connectListNetworks = esxConnectListNetworks, /* 0.10.0 */
@@ -877,4 +942,5 @@ virNetworkDriver esxNetworkDriver = {
 .networkSetAutostart = esxNetworkSetAutostart, /* 0.10.0 */
 .networkIsActive = esxNetworkIsActive, /* 0.10.0 */
 .networkIsPersistent = esxNetworkIsPersistent, /* 0.10.0 */
+.connectListAllNetworks = esxConnectListAllNetworks, /* 5.10.0 */
 };
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v2 08/12] esx: set vmfs fs type for vmfs-based datastores

2019-11-15 Thread Pino Toscano
This way they are correctly represented:
  

  
... instead of 'auto'.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_storage_backend_vmfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/esx/esx_storage_backend_vmfs.c 
b/src/esx/esx_storage_backend_vmfs.c
index 05b273aed7..1270c21e00 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -531,6 +531,7 @@ esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned 
int flags)
 }
 } else if (esxVI_VmfsDatastoreInfo_DynamicCast(info)) {
 def.type = VIR_STORAGE_POOL_FS;
+def.source.format = VIR_STORAGE_POOL_FS_VMFS;
 /*
  * FIXME: I'm not sure how to represent the source and target of a
  * VMFS based datastore in libvirt terms
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v2 00/12] esx: various improvements

2019-11-15 Thread Pino Toscano
- implement connectListAllStoragePools, so
  virConnectListAllStoragePools() works
- implement connectListAllNetworks, so virConnectListAllNetworks()
  works
- implement storagePoolListAllVolumes, so virStoragePoolListAllVolumes()
  works
- set the proper filesystem type for vmfs-based datastores

Pino Toscano (12):
  esx: split datastoreToStoragePoolPtr helper
  esx: split datastorePoolType helper
  esx: split targetToStoragePool helper
  esx: implement connectListAllStoragePools
  esx: split virtualswitchToNetwork helper
  esx: implement connectListAllNetworks
  storage: add vmfs filesystem type
  esx: set vmfs fs type for vmfs-based datastores
  esx: split datastorePathToStorageVol helper
  esx: split scsilunToStorageVol helper
  esx: implement storagePoolListAllVolumes
  docs: document implemented APIs in esx

 docs/news.xml |  11 +
 docs/schemas/storagepool.rng  |   1 +
 docs/schemas/storagevol.rng   |   1 +
 docs/storage.html.in  |   3 +
 src/conf/storage_conf.c   |   1 +
 src/conf/storage_conf.h   |   1 +
 src/esx/esx_network_driver.c  | 100 +-
 src/esx/esx_storage_backend_iscsi.c   | 234 +++---
 src/esx/esx_storage_backend_vmfs.c| 306 +++---
 src/esx/esx_storage_driver.c  |  87 +
 .../storagepoolcapsschemadata/poolcaps-fs.xml |   1 +
 .../poolcaps-full.xml |   1 +
 12 files changed, 655 insertions(+), 92 deletions(-)

-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v2 02/12] esx: split datastorePoolType helper

2019-11-15 Thread Pino Toscano
Move the detection of the type of a vmfs pool out of
esxLookupVMFSStoragePoolType in an own helper. This way it can be used
also in other functions.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_storage_backend_vmfs.c | 49 --
 1 file changed, 33 insertions(+), 16 deletions(-)

diff --git a/src/esx/esx_storage_backend_vmfs.c 
b/src/esx/esx_storage_backend_vmfs.c
index 78fe2b598d..b890825a40 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -54,26 +54,12 @@ verify(VIR_CRYPTO_HASH_SIZE_MD5 == VIR_UUID_BUFLEN);
 
 
 static int
-esxLookupVMFSStoragePoolType(esxVI_Context *ctx, const char *poolName,
- int *poolType)
+datastorePoolType(esxVI_ObjectContent *datastore, int *poolType)
 {
 int result = -1;
-esxVI_String *propertyNameList = NULL;
-esxVI_ObjectContent *datastore = NULL;
 esxVI_DynamicProperty *dynamicProperty = NULL;
 esxVI_DatastoreInfo *datastoreInfo = NULL;
 
-if (esxVI_String_AppendValueToList(, "info") < 0 ||
-esxVI_LookupDatastoreByName(ctx, poolName, propertyNameList, 
,
-esxVI_Occurrence_OptionalItem) < 0) {
-goto cleanup;
-}
-
-if (!datastore) {
-/* Not found, let the base storage driver handle error reporting */
-goto cleanup;
-}
-
 for (dynamicProperty = datastore->propSet; dynamicProperty;
  dynamicProperty = dynamicProperty->_next) {
 if (STREQ(dynamicProperty->name, "info")) {
@@ -100,10 +86,41 @@ esxLookupVMFSStoragePoolType(esxVI_Context *ctx, const 
char *poolName,
 
 result = 0;
 
+ cleanup:
+esxVI_DatastoreInfo_Free();
+
+return result;
+}
+
+
+
+static int
+esxLookupVMFSStoragePoolType(esxVI_Context *ctx, const char *poolName,
+ int *poolType)
+{
+int result = -1;
+esxVI_String *propertyNameList = NULL;
+esxVI_ObjectContent *datastore = NULL;
+
+if (esxVI_String_AppendValueToList(, "info") < 0 ||
+esxVI_LookupDatastoreByName(ctx, poolName, propertyNameList, 
,
+esxVI_Occurrence_OptionalItem) < 0) {
+goto cleanup;
+}
+
+if (!datastore) {
+/* Not found, let the base storage driver handle error reporting */
+goto cleanup;
+}
+
+if (datastorePoolType(datastore, poolType) < 0)
+goto cleanup;
+
+result = 0;
+
  cleanup:
 esxVI_String_Free();
 esxVI_ObjectContent_Free();
-esxVI_DatastoreInfo_Free();
 
 return result;
 }
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v2 09/12] esx: split datastorePathToStorageVol helper

2019-11-15 Thread Pino Toscano
Move the creation of a virStorageVolPtr object by lookup out of
esxStorageVolLookupByPath in an own helper. This way it can be used
also in other functions.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_storage_backend_vmfs.c | 28 ++--
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/esx/esx_storage_backend_vmfs.c 
b/src/esx/esx_storage_backend_vmfs.c
index 1270c21e00..ab47a4c272 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -695,32 +695,40 @@ esxStorageVolLookupByName(virStoragePoolPtr pool,
 
 
 
+static virStorageVolPtr
+datastorePathToStorageVol(virConnectPtr conn, const char *pool,
+  const char *path)
+{
+esxPrivate *priv = conn->privateData;
+g_autofree char *key = NULL;
+
+if (esxVI_LookupStorageVolumeKeyByDatastorePath(priv->primary, path,
+) < 0) {
+return NULL;
+}
+
+return virGetStorageVol(conn, pool, path, key,
+, NULL);
+}
+
+
 static virStorageVolPtr
 esxStorageVolLookupByPath(virConnectPtr conn, const char *path)
 {
 virStorageVolPtr volume = NULL;
-esxPrivate *priv = conn->privateData;
 char *datastoreName = NULL;
 char *directoryAndFileName = NULL;
-char *key = NULL;
 
 if (esxUtil_ParseDatastorePath(path, , NULL,
) < 0) {
 goto cleanup;
 }
 
-if (esxVI_LookupStorageVolumeKeyByDatastorePath(priv->primary, path,
-) < 0) {
-goto cleanup;
-}
-
-volume = virGetStorageVol(conn, datastoreName, directoryAndFileName, key,
-  , NULL);
+volume = datastorePathToStorageVol(conn, datastoreName, 
directoryAndFileName);
 
  cleanup:
 VIR_FREE(datastoreName);
 VIR_FREE(directoryAndFileName);
-VIR_FREE(key);
 
 return volume;
 }
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v2 07/12] storage: add vmfs filesystem type

2019-11-15 Thread Pino Toscano
It will be used to represent the type of a filesystem pool in ESXi.

Signed-off-by: Pino Toscano 
---
 docs/schemas/storagepool.rng  | 1 +
 docs/schemas/storagevol.rng   | 1 +
 docs/storage.html.in  | 3 +++
 src/conf/storage_conf.c   | 1 +
 src/conf/storage_conf.h   | 1 +
 tests/storagepoolcapsschemadata/poolcaps-fs.xml   | 1 +
 tests/storagepoolcapsschemadata/poolcaps-full.xml | 1 +
 7 files changed, 9 insertions(+)

diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index 976a02baeb..ff0d3c836c 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -451,6 +451,7 @@
 hfs+
 xfs
 ocfs2
+vmfs
   
 
   
diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng
index 32aaa2784d..382cd121ad 100644
--- a/docs/schemas/storagevol.rng
+++ b/docs/schemas/storagevol.rng
@@ -194,6 +194,7 @@
   hfs+
   xfs
   ocfs2
+  vmfs
 
   
 
diff --git a/docs/storage.html.in b/docs/storage.html.in
index e0e4edec1e..72fa13944a 100644
--- a/docs/storage.html.in
+++ b/docs/storage.html.in
@@ -202,6 +202,9 @@
   
 ocfs2
   
+  
+vmfs
+  
 
 
 Valid volume format types
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index fd640bfa2b..252d28cbfb 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -67,6 +67,7 @@ VIR_ENUM_IMPL(virStoragePoolFormatFileSystem,
   "auto", "ext2", "ext3",
   "ext4", "ufs", "iso9660", "udf",
   "gfs", "gfs2", "vfat", "hfs+", "xfs", "ocfs2",
+  "vmfs",
 );
 
 VIR_ENUM_IMPL(virStoragePoolFormatFileSystemNet,
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index d2600efff0..c0baeffc1c 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -362,6 +362,7 @@ typedef enum {
 VIR_STORAGE_POOL_FS_HFSPLUS,
 VIR_STORAGE_POOL_FS_XFS,
 VIR_STORAGE_POOL_FS_OCFS2,
+VIR_STORAGE_POOL_FS_VMFS,
 VIR_STORAGE_POOL_FS_LAST,
 } virStoragePoolFormatFileSystem;
 VIR_ENUM_DECL(virStoragePoolFormatFileSystem);
diff --git a/tests/storagepoolcapsschemadata/poolcaps-fs.xml 
b/tests/storagepoolcapsschemadata/poolcaps-fs.xml
index 182fa398f5..eee75af746 100644
--- a/tests/storagepoolcapsschemadata/poolcaps-fs.xml
+++ b/tests/storagepoolcapsschemadata/poolcaps-fs.xml
@@ -40,6 +40,7 @@
 hfs+
 xfs
 ocfs2
+vmfs
   
 
 
diff --git a/tests/storagepoolcapsschemadata/poolcaps-full.xml 
b/tests/storagepoolcapsschemadata/poolcaps-full.xml
index 980c6d210e..805950a937 100644
--- a/tests/storagepoolcapsschemadata/poolcaps-full.xml
+++ b/tests/storagepoolcapsschemadata/poolcaps-full.xml
@@ -40,6 +40,7 @@
 hfs+
 xfs
 ocfs2
+vmfs
   
 
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v2 01/12] esx: split datastoreToStoragePoolPtr helper

2019-11-15 Thread Pino Toscano
Move the creation of a virStoragePtr object from the esxVI_ObjectContent
object of a datastore out of esxStoragePoolLookupByName in an own
helper. This way it can be used also in other functions.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_storage_backend_vmfs.c | 45 --
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/src/esx/esx_storage_backend_vmfs.c 
b/src/esx/esx_storage_backend_vmfs.c
index 5f25f80072..78fe2b598d 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -195,26 +195,16 @@ esxConnectListStoragePools(virConnectPtr conn, char 
**const names,
 
 
 static virStoragePoolPtr
-esxStoragePoolLookupByName(virConnectPtr conn,
-   const char *name)
+datastoreToStoragePoolPtr(virConnectPtr conn,
+  const char *name,
+  esxVI_ObjectContent *datastore)
 {
 esxPrivate *priv = conn->privateData;
-esxVI_ObjectContent *datastore = NULL;
 esxVI_DatastoreHostMount *hostMount = NULL;
 /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */
 unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5];
 virStoragePoolPtr pool = NULL;
 
-if (esxVI_LookupDatastoreByName(priv->primary, name, NULL, ,
-esxVI_Occurrence_OptionalItem) < 0) {
-goto cleanup;
-}
-
-if (!datastore) {
-/* Not found, let the base storage driver handle error reporting */
-goto cleanup;
-}
-
 /*
  * Datastores don't have a UUID, but we can use the 'host.mountInfo.path'
  * property as source for a UUID. The mount path is unique per host and
@@ -239,7 +229,6 @@ esxStoragePoolLookupByName(virConnectPtr conn,
 pool = virGetStoragePool(conn, name, md5, , NULL);
 
  cleanup:
-esxVI_ObjectContent_Free();
 esxVI_DatastoreHostMount_Free();
 
 return pool;
@@ -247,6 +236,34 @@ esxStoragePoolLookupByName(virConnectPtr conn,
 
 
 
+static virStoragePoolPtr
+esxStoragePoolLookupByName(virConnectPtr conn,
+   const char *name)
+{
+esxPrivate *priv = conn->privateData;
+esxVI_ObjectContent *datastore = NULL;
+virStoragePoolPtr pool = NULL;
+
+if (esxVI_LookupDatastoreByName(priv->primary, name, NULL, ,
+esxVI_Occurrence_OptionalItem) < 0) {
+goto cleanup;
+}
+
+if (!datastore) {
+/* Not found, let the base storage driver handle error reporting */
+goto cleanup;
+}
+
+pool = datastoreToStoragePoolPtr(conn, name, datastore);
+
+ cleanup:
+esxVI_ObjectContent_Free();
+
+return pool;
+}
+
+
+
 static virStoragePoolPtr
 esxStoragePoolLookupByUUID(virConnectPtr conn,
const unsigned char *uuid)
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v2 10/12] esx: split scsilunToStorageVol helper

2019-11-15 Thread Pino Toscano
Move the creation of a virStorageVolPtr object from the esxVI_ScsiLun
object of a SCSI lun out of esxStorageVolLookupByName and
esxStorageVolLookupByPath in an own helper. This way it can be used
also in other functions.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_storage_backend_iscsi.c | 60 +++--
 1 file changed, 31 insertions(+), 29 deletions(-)

diff --git a/src/esx/esx_storage_backend_iscsi.c 
b/src/esx/esx_storage_backend_iscsi.c
index 4f5d8e5e24..34760a837e 100644
--- a/src/esx/esx_storage_backend_iscsi.c
+++ b/src/esx/esx_storage_backend_iscsi.c
@@ -440,6 +440,35 @@ esxStoragePoolListVolumes(virStoragePoolPtr pool, char 
**const names,
 
 
 
+static virStorageVolPtr
+scsilunToStorageVol(virConnectPtr conn, esxVI_ScsiLun *scsiLun,
+const char *pool)
+{
+/* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */
+unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5];
+char uuid_string[VIR_UUID_STRING_BUFLEN] = "";
+
+/*
+ * ScsiLun provides a UUID field that is unique across
+ * multiple servers. But this field length is ~55 characters
+ * compute MD5 hash to transform it to an acceptable
+ * libvirt format
+ */
+if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, scsiLun->uuid, md5) < 0)
+return NULL;
+virUUIDFormat(md5, uuid_string);
+
+/*
+ * ScsiLun provides displayName and canonicalName but both are
+ * optional and its observed that they can be NULL, using
+ * deviceName to create volume.
+ */
+return virGetStorageVol(conn, pool, scsiLun->deviceName, uuid_string,
+, NULL);
+}
+
+
+
 static virStorageVolPtr
 esxStorageVolLookupByName(virStoragePoolPtr pool,
   const char *name)
@@ -448,9 +477,6 @@ esxStorageVolLookupByName(virStoragePoolPtr pool,
 esxPrivate *priv = pool->conn->privateData;
 esxVI_ScsiLun *scsiLunList = NULL;
 esxVI_ScsiLun *scsiLun;
-/* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */
-unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5];
-char uuid_string[VIR_UUID_STRING_BUFLEN] = "";
 
 if (esxVI_LookupScsiLunList(priv->primary, ) < 0)
 goto cleanup;
@@ -458,23 +484,7 @@ esxStorageVolLookupByName(virStoragePoolPtr pool,
 for (scsiLun = scsiLunList; scsiLun;
  scsiLun = scsiLun->_next) {
 if (STREQ(scsiLun->deviceName, name)) {
-/*
- * ScsiLun provides a UUID field that is unique across
- * multiple servers. But this field length is ~55 characters
- * compute MD5 hash to transform it to an acceptable
- * libvirt format
- */
-if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, scsiLun->uuid, md5) < 0)
-goto cleanup;
-virUUIDFormat(md5, uuid_string);
-
-/*
- * ScsiLun provides displayName and canonicalName but both are
- * optional and its observed that they can be NULL, using
- * deviceName to create volume.
- */
-volume = virGetStorageVol(pool->conn, pool->name, name, 
uuid_string,
-  , NULL);
+volume = scsilunToStorageVol(pool->conn, scsiLun, pool->name);
 break;
 }
 }
@@ -496,9 +506,6 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char 
*path)
 esxVI_ScsiLun *scsiLun;
 esxVI_HostScsiDisk *hostScsiDisk = NULL;
 char *poolName = NULL;
-/* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */
-unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5];
-char uuid_string[VIR_UUID_STRING_BUFLEN] = "";
 
 if (esxVI_LookupScsiLunList(priv->primary, ) < 0)
 goto cleanup;
@@ -516,12 +523,7 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char 
*path)
 goto cleanup;
 }
 
-if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, scsiLun->uuid, md5) < 0)
-goto cleanup;
-virUUIDFormat(md5, uuid_string);
-
-volume = virGetStorageVol(conn, poolName, path, uuid_string,
-  , NULL);
+volume = scsilunToStorageVol(conn, scsiLun, poolName);
 break;
 }
 }
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v2 11/12] esx: implement storagePoolListAllVolumes

2019-11-15 Thread Pino Toscano
Implement the .storagePoolListAllVolumes storage API in the esx storage
driver, and in all its subdrivers.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_storage_backend_iscsi.c | 70 
 src/esx/esx_storage_backend_vmfs.c  | 85 +
 src/esx/esx_storage_driver.c| 19 +++
 3 files changed, 174 insertions(+)

diff --git a/src/esx/esx_storage_backend_iscsi.c 
b/src/esx/esx_storage_backend_iscsi.c
index 34760a837e..2f189a92cf 100644
--- a/src/esx/esx_storage_backend_iscsi.c
+++ b/src/esx/esx_storage_backend_iscsi.c
@@ -852,6 +852,75 @@ esxConnectListAllStoragePools(virConnectPtr conn,
 
 
 
+static int
+esxStoragePoolListAllVolumes(virStoragePoolPtr pool,
+ virStorageVolPtr **vols,
+ unsigned int flags)
+{
+bool success = false;
+size_t count = 0;
+esxPrivate *priv = pool->conn->privateData;
+esxVI_HostScsiTopologyLun *hostScsiTopologyLunList = NULL;
+esxVI_HostScsiTopologyLun *hostScsiTopologyLun;
+esxVI_ScsiLun *scsiLunList = NULL;
+esxVI_ScsiLun *scsiLun = NULL;
+size_t i;
+
+virCheckFlags(0, -1);
+
+if (esxVI_LookupHostScsiTopologyLunListByTargetName
+  (priv->primary, pool->name, ) < 0) {
+goto cleanup;
+}
+
+if (!hostScsiTopologyLunList) {
+/* iSCSI adapter may not be enabled on ESX host */
+return 0;
+}
+
+if (esxVI_LookupScsiLunList(priv->primary, ) < 0)
+goto cleanup;
+
+for (scsiLun = scsiLunList; scsiLun;
+ scsiLun = scsiLun->_next) {
+for (hostScsiTopologyLun = hostScsiTopologyLunList;
+ hostScsiTopologyLun;
+ hostScsiTopologyLun = hostScsiTopologyLun->_next) {
+if (STREQ(hostScsiTopologyLun->scsiLun, scsiLun->key)) {
+virStorageVolPtr volume;
+
+volume = scsilunToStorageVol(pool->conn, scsiLun, pool->name);
+if (!volume)
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(*vols, count, volume) < 0)
+goto cleanup;
+}
+
+}
+}
+
+success = true;
+
+ cleanup:
+if (! success) {
+if (*vols) {
+for (i = 0; i < count; ++i)
+VIR_FREE((*vols)[i]);
+VIR_FREE(*vols);
+}
+
+count = -1;
+}
+
+esxVI_HostScsiTopologyLun_Free();
+esxVI_ScsiLun_Free();
+
+return count;
+}
+
+
+
 virStorageDriver esxStorageBackendISCSI = {
 .connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 1.0.1 */
 .connectListStoragePools = esxConnectListStoragePools, /* 1.0.1 */
@@ -873,4 +942,5 @@ virStorageDriver esxStorageBackendISCSI = {
 .storageVolWipe = esxStorageVolWipe, /* 1.0.1 */
 .storageVolGetPath = esxStorageVolGetPath, /* 1.0.1 */
 .connectListAllStoragePools = esxConnectListAllStoragePools, /* 5.10.0 */
+.storagePoolListAllVolumes = esxStoragePoolListAllVolumes, /* 5.10.0 */
 };
diff --git a/src/esx/esx_storage_backend_vmfs.c 
b/src/esx/esx_storage_backend_vmfs.c
index ab47a4c272..27f8b17e06 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -1566,6 +1566,90 @@ esxConnectListAllStoragePools(virConnectPtr conn,
 
 
 
+static int
+esxStoragePoolListAllVolumes(virStoragePoolPtr pool,
+ virStorageVolPtr **vols,
+ unsigned int flags)
+{
+bool success = false;
+esxPrivate *priv = pool->conn->privateData;
+esxVI_HostDatastoreBrowserSearchResults *searchResultsList = NULL;
+esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL;
+esxVI_FileInfo *fileInfo = NULL;
+char *directoryAndFileName = NULL;
+size_t length;
+size_t count = 0;
+size_t i;
+
+virCheckFlags(0, -1);
+
+if (esxVI_LookupDatastoreContentByDatastoreName(priv->primary, pool->name,
+) < 0) {
+goto cleanup;
+}
+
+/* Interpret search result */
+for (searchResults = searchResultsList; searchResults;
+ searchResults = searchResults->_next) {
+VIR_FREE(directoryAndFileName);
+
+if (esxUtil_ParseDatastorePath(searchResults->folderPath, NULL, NULL,
+   ) < 0) {
+goto cleanup;
+}
+
+/* Strip trailing separators */
+length = strlen(directoryAndFileName);
+
+while (length > 0 && directoryAndFileName[length - 1] == '/') {
+directoryAndFileName[length - 1] = '\0';
+--length;
+}
+
+/* Build volume names */
+for (fileInfo = searchResults->file; fileInfo;
+ fileInfo = fileInfo->_next) {
+g_autofree char *datastorePath = NULL;
+virStorageVolPtr volume;
+
+if (length < 1) {
+datas

[libvirt] [PATCH v2 03/12] esx: split targetToStoragePool helper

2019-11-15 Thread Pino Toscano
Move the creation of a virStoragePtr object from the
esxVI_HostInternetScsiHbaStaticTarget object of a target out of
esxStoragePoolLookupByName in an own helper. This way it can be used
also in other functions.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_storage_backend_iscsi.c | 32 +++--
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/src/esx/esx_storage_backend_iscsi.c 
b/src/esx/esx_storage_backend_iscsi.c
index 61354a6938..72ab0d3cb0 100644
--- a/src/esx/esx_storage_backend_iscsi.c
+++ b/src/esx/esx_storage_backend_iscsi.c
@@ -147,14 +147,32 @@ esxConnectListStoragePools(virConnectPtr conn, char 
**const names,
 
 
 
+static virStoragePoolPtr
+targetToStoragePool(virConnectPtr conn,
+const char *name,
+esxVI_HostInternetScsiHbaStaticTarget *target)
+{
+/* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */
+unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5];
+
+/*
+ * HostInternetScsiHbaStaticTarget does not provide a uuid field,
+ * but iScsiName (or widely known as IQN) is unique across the multiple
+ * hosts, using it to compute key
+ */
+if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, target->iScsiName, md5) < 0)
+return NULL;
+
+return virGetStoragePool(conn, name, md5, , NULL);
+}
+
+
 static virStoragePoolPtr
 esxStoragePoolLookupByName(virConnectPtr conn,
const char *name)
 {
 esxPrivate *priv = conn->privateData;
 esxVI_HostInternetScsiHbaStaticTarget *target = NULL;
-/* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */
-unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5];
 virStoragePoolPtr pool = NULL;
 
 /*
@@ -172,15 +190,7 @@ esxStoragePoolLookupByName(virConnectPtr conn,
 goto cleanup;
 }
 
-/*
- * HostInternetScsiHbaStaticTarget does not provide a uuid field,
- * but iScsiName (or widely known as IQN) is unique across the multiple
- * hosts, using it to compute key
- */
-if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, target->iScsiName, md5) < 0)
-goto cleanup;
-
-pool = virGetStoragePool(conn, name, md5, , NULL);
+pool = targetToStoragePool(conn, name, target);
 
  cleanup:
 esxVI_HostInternetScsiHbaStaticTarget_Free();
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH 0/5] esx: implement virConnectListAllStoragePools

2019-11-15 Thread Pino Toscano
On Wednesday, 13 November 2019 13:46:06 CET Pino Toscano wrote:
> Make the virConnectListAllStoragePools API work for esx storage pools by
> implementing the internal connectListAllStoragePools storage API.
> 
> Pino Toscano (5):
>   esx: split datastoreToStoragePoolPtr helper
>   esx: split datastorePoolType helper
>   esx: split targetToStoragePool helper
>   esx: implement connectListAllStoragePools
>   docs: document virConnectListAllStoragePools in esx
> 
>  docs/news.xml   |   9 ++
>  src/esx/esx_storage_backend_iscsi.c | 104 +--
>  src/esx/esx_storage_backend_vmfs.c  | 192 +++-
>  src/esx/esx_storage_driver.c|  68 ++
>  4 files changed, 332 insertions(+), 41 deletions(-)

I added more esx-related changes, so I will submit a v2 of this series.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] docs: mention lifted vCPUs restriction for esx

2019-11-13 Thread Pino Toscano
It was lifted with c92b6023e8eb670e01571e299a85e9da9bd4844c.

Signed-off-by: Pino Toscano 
---
 docs/drvesx.html.in | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in
index 901d65958a..ac7bc645d1 100644
--- a/docs/drvesx.html.in
+++ b/docs/drvesx.html.in
@@ -337,7 +337,9 @@ error: invalid argument in libvirt was built without the 
'esx' driver
 Memory size has to be a multiple of 4096
 
 
-Number of virtual CPU has to be 1 or a multiple of 2
+Number of virtual CPU has to be 1 or a multiple of 2.
+Since 4.10.0 any number of vCPUs is
+supported.
 
 
 Valid MAC address prefixes are 00:0c:29 and
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH 2/5] esx: split datastorePoolType helper

2019-11-13 Thread Pino Toscano
Move the detection of the type of a vmfs pool out of
esxLookupVMFSStoragePoolType in an own helper. This way it can be used
also in other functions.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_storage_backend_vmfs.c | 49 --
 1 file changed, 33 insertions(+), 16 deletions(-)

diff --git a/src/esx/esx_storage_backend_vmfs.c 
b/src/esx/esx_storage_backend_vmfs.c
index 78fe2b598d..b890825a40 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -54,26 +54,12 @@ verify(VIR_CRYPTO_HASH_SIZE_MD5 == VIR_UUID_BUFLEN);
 
 
 static int
-esxLookupVMFSStoragePoolType(esxVI_Context *ctx, const char *poolName,
- int *poolType)
+datastorePoolType(esxVI_ObjectContent *datastore, int *poolType)
 {
 int result = -1;
-esxVI_String *propertyNameList = NULL;
-esxVI_ObjectContent *datastore = NULL;
 esxVI_DynamicProperty *dynamicProperty = NULL;
 esxVI_DatastoreInfo *datastoreInfo = NULL;
 
-if (esxVI_String_AppendValueToList(, "info") < 0 ||
-esxVI_LookupDatastoreByName(ctx, poolName, propertyNameList, 
,
-esxVI_Occurrence_OptionalItem) < 0) {
-goto cleanup;
-}
-
-if (!datastore) {
-/* Not found, let the base storage driver handle error reporting */
-goto cleanup;
-}
-
 for (dynamicProperty = datastore->propSet; dynamicProperty;
  dynamicProperty = dynamicProperty->_next) {
 if (STREQ(dynamicProperty->name, "info")) {
@@ -100,10 +86,41 @@ esxLookupVMFSStoragePoolType(esxVI_Context *ctx, const 
char *poolName,
 
 result = 0;
 
+ cleanup:
+esxVI_DatastoreInfo_Free();
+
+return result;
+}
+
+
+
+static int
+esxLookupVMFSStoragePoolType(esxVI_Context *ctx, const char *poolName,
+ int *poolType)
+{
+int result = -1;
+esxVI_String *propertyNameList = NULL;
+esxVI_ObjectContent *datastore = NULL;
+
+if (esxVI_String_AppendValueToList(, "info") < 0 ||
+esxVI_LookupDatastoreByName(ctx, poolName, propertyNameList, 
,
+esxVI_Occurrence_OptionalItem) < 0) {
+goto cleanup;
+}
+
+if (!datastore) {
+/* Not found, let the base storage driver handle error reporting */
+goto cleanup;
+}
+
+if (datastorePoolType(datastore, poolType) < 0)
+goto cleanup;
+
+result = 0;
+
  cleanup:
 esxVI_String_Free();
 esxVI_ObjectContent_Free();
-esxVI_DatastoreInfo_Free();
 
 return result;
 }
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH 4/5] esx: implement connectListAllStoragePools

2019-11-13 Thread Pino Toscano
Implement the .connectListAllStoragePools storage API in the esx storage
driver, and in all its subdrivers.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_storage_backend_iscsi.c | 72 +
 src/esx/esx_storage_backend_vmfs.c  | 98 +
 src/esx/esx_storage_driver.c| 68 
 3 files changed, 238 insertions(+)

diff --git a/src/esx/esx_storage_backend_iscsi.c 
b/src/esx/esx_storage_backend_iscsi.c
index 72ab0d3cb0..4f5d8e5e24 100644
--- a/src/esx/esx_storage_backend_iscsi.c
+++ b/src/esx/esx_storage_backend_iscsi.c
@@ -779,6 +779,77 @@ esxStorageVolGetPath(virStorageVolPtr volume)
 
 
 
+#define MATCH(FLAG) (flags & (FLAG))
+static int
+esxConnectListAllStoragePools(virConnectPtr conn,
+  virStoragePoolPtr **pools,
+  unsigned int flags)
+{
+bool success = false;
+size_t count = 0;
+esxPrivate *priv = conn->privateData;
+esxVI_HostInternetScsiHba *hostInternetScsiHba = NULL;
+esxVI_HostInternetScsiHbaStaticTarget *target;
+size_t i;
+
+/* this driver provides only iSCSI pools */
+if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE) &&
+!(MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_ISCSI)))
+return 0;
+
+if (esxVI_LookupHostInternetScsiHba(priv->primary,
+) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Unable to obtain iSCSI adapter"));
+goto cleanup;
+}
+
+/* FIXME: code looks for software iSCSI adapter only */
+if (!hostInternetScsiHba) {
+/* iSCSI adapter may not be enabled for this host */
+return 0;
+}
+
+/*
+ * ESX has two kind of targets:
+ * 1. staticIscsiTargets
+ * 2. dynamicIscsiTargets
+ * For each dynamic target if its reachable a static target is added.
+ * return iSCSI names for all static targets to avoid duplicate names.
+ */
+for (target = hostInternetScsiHba->configuredStaticTarget;
+ target; target = target->_next) {
+virStoragePoolPtr pool;
+
+pool = targetToStoragePool(conn, target->iScsiName, target);
+if (!pool)
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(*pools, count, pool) < 0)
+goto cleanup;
+}
+
+success = true;
+
+ cleanup:
+if (! success) {
+if (*pools) {
+for (i = 0; i < count; ++i)
+VIR_FREE((*pools)[i]);
+VIR_FREE(*pools);
+}
+
+count = -1;
+}
+
+esxVI_HostInternetScsiHba_Free();
+
+return count;
+}
+#undef MATCH
+
+
+
 virStorageDriver esxStorageBackendISCSI = {
 .connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 1.0.1 */
 .connectListStoragePools = esxConnectListStoragePools, /* 1.0.1 */
@@ -799,4 +870,5 @@ virStorageDriver esxStorageBackendISCSI = {
 .storageVolDelete = esxStorageVolDelete, /* 1.0.1 */
 .storageVolWipe = esxStorageVolWipe, /* 1.0.1 */
 .storageVolGetPath = esxStorageVolGetPath, /* 1.0.1 */
+.connectListAllStoragePools = esxConnectListAllStoragePools, /* 5.10.0 */
 };
diff --git a/src/esx/esx_storage_backend_vmfs.c 
b/src/esx/esx_storage_backend_vmfs.c
index b890825a40..05b273aed7 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -1460,6 +1460,103 @@ esxStorageVolGetPath(virStorageVolPtr volume)
 
 
 
+#define MATCH(FLAG) (flags & (FLAG))
+static int
+esxConnectListAllStoragePools(virConnectPtr conn,
+  virStoragePoolPtr **pools,
+  unsigned int flags)
+{
+bool success = false;
+esxPrivate *priv = conn->privateData;
+esxVI_String *propertyNameList = NULL;
+esxVI_DynamicProperty *dynamicProperty = NULL;
+esxVI_ObjectContent *datastoreList = NULL;
+esxVI_ObjectContent *datastore = NULL;
+size_t count = 0;
+size_t i;
+virStoragePoolPtr pool;
+const bool checkPoolType = 
MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE);
+
+if (esxVI_String_AppendValueToList(,
+   "summary.name") < 0) {
+goto cleanup;
+}
+
+if (checkPoolType &&
+esxVI_String_AppendValueToList(,
+   "info") < 0) {
+goto cleanup;
+}
+
+if (esxVI_LookupDatastoreList(priv->primary, propertyNameList,
+  ) < 0) {
+goto cleanup;
+}
+
+for (datastore = datastoreList; datastore;
+ datastore = datastore->_next) {
+const char *name = NULL;
+
+for (dynamicProperty = datastore->propSet; dynamicProperty;
+ dynamicProperty = dynamicProperty->_next) {
+if (STREQ(dynamicProperty->name, "summary.name")) {
+  

[libvirt] [PATCH 0/5] esx: implement virConnectListAllStoragePools

2019-11-13 Thread Pino Toscano
Make the virConnectListAllStoragePools API work for esx storage pools by
implementing the internal connectListAllStoragePools storage API.

Pino Toscano (5):
  esx: split datastoreToStoragePoolPtr helper
  esx: split datastorePoolType helper
  esx: split targetToStoragePool helper
  esx: implement connectListAllStoragePools
  docs: document virConnectListAllStoragePools in esx

 docs/news.xml   |   9 ++
 src/esx/esx_storage_backend_iscsi.c | 104 +--
 src/esx/esx_storage_backend_vmfs.c  | 192 +++-
 src/esx/esx_storage_driver.c|  68 ++
 4 files changed, 332 insertions(+), 41 deletions(-)

-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH 1/5] esx: split datastoreToStoragePoolPtr helper

2019-11-13 Thread Pino Toscano
Move the creation of a virStoragePtr object from the esxVI_ObjectContent
object of a datastore out of esxStoragePoolLookupByName in an own
helper. This way it can be used also in other functions.

Signed-off-by: Pino Toscano 
---
 src/esx/esx_storage_backend_vmfs.c | 45 --
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/src/esx/esx_storage_backend_vmfs.c 
b/src/esx/esx_storage_backend_vmfs.c
index 5f25f80072..78fe2b598d 100644
--- a/src/esx/esx_storage_backend_vmfs.c
+++ b/src/esx/esx_storage_backend_vmfs.c
@@ -195,26 +195,16 @@ esxConnectListStoragePools(virConnectPtr conn, char 
**const names,
 
 
 static virStoragePoolPtr
-esxStoragePoolLookupByName(virConnectPtr conn,
-   const char *name)
+datastoreToStoragePoolPtr(virConnectPtr conn,
+  const char *name,
+  esxVI_ObjectContent *datastore)
 {
 esxPrivate *priv = conn->privateData;
-esxVI_ObjectContent *datastore = NULL;
 esxVI_DatastoreHostMount *hostMount = NULL;
 /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */
 unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5];
 virStoragePoolPtr pool = NULL;
 
-if (esxVI_LookupDatastoreByName(priv->primary, name, NULL, ,
-esxVI_Occurrence_OptionalItem) < 0) {
-goto cleanup;
-}
-
-if (!datastore) {
-/* Not found, let the base storage driver handle error reporting */
-goto cleanup;
-}
-
 /*
  * Datastores don't have a UUID, but we can use the 'host.mountInfo.path'
  * property as source for a UUID. The mount path is unique per host and
@@ -239,7 +229,6 @@ esxStoragePoolLookupByName(virConnectPtr conn,
 pool = virGetStoragePool(conn, name, md5, , NULL);
 
  cleanup:
-esxVI_ObjectContent_Free();
 esxVI_DatastoreHostMount_Free();
 
 return pool;
@@ -247,6 +236,34 @@ esxStoragePoolLookupByName(virConnectPtr conn,
 
 
 
+static virStoragePoolPtr
+esxStoragePoolLookupByName(virConnectPtr conn,
+   const char *name)
+{
+esxPrivate *priv = conn->privateData;
+esxVI_ObjectContent *datastore = NULL;
+virStoragePoolPtr pool = NULL;
+
+if (esxVI_LookupDatastoreByName(priv->primary, name, NULL, ,
+esxVI_Occurrence_OptionalItem) < 0) {
+goto cleanup;
+}
+
+if (!datastore) {
+/* Not found, let the base storage driver handle error reporting */
+goto cleanup;
+}
+
+pool = datastoreToStoragePoolPtr(conn, name, datastore);
+
+ cleanup:
+esxVI_ObjectContent_Free();
+
+return pool;
+}
+
+
+
 static virStoragePoolPtr
 esxStoragePoolLookupByUUID(virConnectPtr conn,
const unsigned char *uuid)
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



  1   2   3   >