12:46 p.m.
Since v1:
- use syntax-check rather than dynamic runtime check for API mismatch
- fix more stragglers with mismatched API, found by the syntax-check
- fix a bug in bhyve no-op flag handling
- expand no-op flag handling to other affected drivers
Eric Blake (9):
vbox: Add various vir*Flags API
xenapi: Add various vir*Flags API
maint: Enhance check-driverimpls.pl to check for API pairing
bhyve: Ignore no-op flag during virDomainUndefine
libxl: Ignore no-op flag during virDomainUndefine
lxc: Ignore no-op flag during virDomainUndefine
openvz: Ignore no-op flag during virDomainUndefine
vmware: Ignore no-op flag during virDomainUndefine
xenapi: Ignore no-op flag during virDomainUndefine
src/bhyve/bhyve_driver.c | 6 +++++-
src/check-driverimpls.pl | 33 +++++++++++++++++++++++++++++++--
src/libxl/libxl_driver.c | 4 +++-
src/lxc/lxc_driver.c | 5 ++++-
src/openvz/openvz_driver.c | 5 ++++-
src/vbox/vbox_common.c | 24 ++++++++++++++++++++++--
src/vmware/vmware_driver.c | 5 ++++-
src/xenapi/xenapi_driver.c | 28 ++++++++++++++++++++++++----
8 files changed, 97 insertions(+), 13 deletions(-)
--
2.20.1
12:46 p.m.
New subject: [libvirt] [PATCH v2 1/9] vbox: Add various vir*Flags API
Even though we don't accept any flags, it is unfriendly to callers
that use the modern API to have to fall back to the flag-free API.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/vbox/vbox_common.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 54e31bec9d..44c98cadf6 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -553,7 +553,8 @@ static int vboxConnectClose(virConnectPtr conn)
}
static int
-vboxDomainSave(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED)
+vboxDomainSaveFlags(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED,
+ const char *dxml, unsigned int flags)
{
vboxDriverPtr data = dom->conn->privateData;
IConsole *console = NULL;
@@ -564,6 +565,9 @@ vboxDomainSave(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED)
nsresult rc;
int ret = -1;
+ virCheckFlags(0, -1);
+ virCheckNonNullArgReturn(dxml, -1);
+
if (!data->vboxObj)
return ret;
@@ -607,6 +611,12 @@ vboxDomainSave(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED)
return ret;
}
+static int
+vboxDomainSave(virDomainPtr dom, const char *path)
+{
+ return vboxDomainSaveFlags(dom, path, NULL, 0);
+}
+
static int vboxConnectGetVersion(virConnectPtr conn, unsigned long *version)
{
vboxDriverPtr data = conn->privateData;
@@ -2717,7 +2727,8 @@ static char *vboxDomainGetOSType(virDomainPtr dom ATTRIBUTE_UNUSED)
{
return osType;
}
-static int vboxDomainSetMemory(virDomainPtr dom, unsigned long memory)
+static int vboxDomainSetMemoryFlags(virDomainPtr dom, unsigned long memory,
+ unsigned int flags)
{
vboxDriverPtr data = dom->conn->privateData;
IMachine *machine = NULL;
@@ -2727,6 +2738,8 @@ static int vboxDomainSetMemory(virDomainPtr dom, unsigned long
memory)
nsresult rc;
int ret = -1;
+ virCheckFlags(0, -1);
+
if (!data->vboxObj)
return ret;
@@ -2775,6 +2788,11 @@ static int vboxDomainSetMemory(virDomainPtr dom, unsigned long
memory)
return ret;
}
+static int vboxDomainSetMemory(virDomainPtr dom, unsigned long memory)
+{
+ return vboxDomainSetMemoryFlags(dom, memory, 0);
+}
+
static int vboxDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info)
{
vboxDriverPtr data = dom->conn->privateData;
@@ -7995,9 +8013,11 @@ static virHypervisorDriver vboxCommonDriver = {
.domainDestroyFlags = vboxDomainDestroyFlags, /* 0.9.4 */
.domainGetOSType = vboxDomainGetOSType, /* 0.6.3 */
.domainSetMemory = vboxDomainSetMemory, /* 0.6.3 */
+ .domainSetMemoryFlags = vboxDomainSetMemoryFlags, /* 5.6.0 */
.domainGetInfo = vboxDomainGetInfo, /* 0.6.3 */
.domainGetState = vboxDomainGetState, /* 0.9.2 */
.domainSave = vboxDomainSave, /* 0.6.3 */
+ .domainSaveFlags = vboxDomainSaveFlags, /* 5.6.0 */
.domainSetVcpus = vboxDomainSetVcpus, /* 0.7.1 */
.domainSetVcpusFlags = vboxDomainSetVcpusFlags, /* 0.8.5 */
.domainGetVcpusFlags = vboxDomainGetVcpusFlags, /* 0.8.5 */
--
2.20.1
2:02 a.m.
New subject: [libvirt] [PATCH v2 1/9] vbox: Add various vir*Flags API
On Tue, Jul 09, 2019 at 12:46:30 -0500, Eric Blake wrote:
Even though we don't accept any flags, it is unfriendly to
callers
that use the modern API to have to fall back to the flag-free API.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/vbox/vbox_common.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 54e31bec9d..44c98cadf6 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -553,7 +553,8 @@ static int vboxConnectClose(virConnectPtr conn)
}
static int
-vboxDomainSave(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED)
+vboxDomainSaveFlags(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED,
+ const char *dxml, unsigned int flags)
{
vboxDriverPtr data = dom->conn->privateData;
IConsole *console = NULL;
@@ -564,6 +565,9 @@ vboxDomainSave(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED)
nsresult rc;
int ret = -1;
+ virCheckFlags(0, -1);
+ virCheckNonNullArgReturn(dxml, -1);
This reports: invalid argument: dxml in vboxDomainSave must not be NULL
I'm not certain that the internal function name makes sense to external
users.
+
if (!data->vboxObj)
return ret;
@@ -607,6 +611,12 @@ vboxDomainSave(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED)
return ret;
}
+static int
+vboxDomainSave(virDomainPtr dom, const char *path)
+{
+ return vboxDomainSaveFlags(dom, path, NULL, 0);
So, this passes NULL 'dxml' into vboxDomainSaveFlags which explicitly
rejects it. What's the point?
Ah I see. Was the above supposed to be virCheckNullArgGoto?
6:50 a.m.
New subject: [libvirt] [PATCH v2 1/9] vbox: Add various vir*Flags API
On 7/10/19 2:02 AM, Peter Krempa wrote:
On Tue, Jul 09, 2019 at 12:46:30 -0500, Eric Blake wrote:
> Even though we don't accept any flags, it is unfriendly to callers
> that use the modern API to have to fall back to the flag-free API.
>
> Signed-off-by: Eric Blake <eblake(a)redhat.com>
> ---
> src/vbox/vbox_common.c | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
> index 54e31bec9d..44c98cadf6 100644
> --- a/src/vbox/vbox_common.c
> +++ b/src/vbox/vbox_common.c
> @@ -553,7 +553,8 @@ static int vboxConnectClose(virConnectPtr conn)
> }
>
> static int
> -vboxDomainSave(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED)
> +vboxDomainSaveFlags(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED,
> + const char *dxml, unsigned int flags)
> {
> vboxDriverPtr data = dom->conn->privateData;
> IConsole *console = NULL;
> @@ -564,6 +565,9 @@ vboxDomainSave(virDomainPtr dom, const char *path
ATTRIBUTE_UNUSED)
> nsresult rc;
> int ret = -1;
>
> + virCheckFlags(0, -1);
> + virCheckNonNullArgReturn(dxml, -1);
This reports: invalid argument: dxml in vboxDomainSave must not be NULL
I'm not certain that the internal function name makes sense to external
users.
In which case I can hand-roll a more specific error instead of reusing
the common macro. But I do see pre-existing uses of
virCheckNonNullArgXXX in src/esx and src/vz prior to this patch, so it's
not the first time we've used the common macros.
> +
> if (!data->vboxObj)
> return ret;
>
> @@ -607,6 +611,12 @@ vboxDomainSave(virDomainPtr dom, const char *path
ATTRIBUTE_UNUSED)
> return ret;
> }
>
> +static int
> +vboxDomainSave(virDomainPtr dom, const char *path)
> +{
> + return vboxDomainSaveFlags(dom, path, NULL, 0);
So, this passes NULL 'dxml' into vboxDomainSaveFlags which explicitly
rejects it. What's the point?
Ah I see. Was the above supposed to be virCheckNullArgGoto?
D'oh. Yes, I got it backwards. The function wants to succeed only if
the user omitted the optional dxml argument.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
10:57 a.m.
New subject: [libvirt] [PATCH v2 1/9] vbox: Add various vir*Flags API
On 7/10/19 6:50 AM, Eric Blake wrote:
On 7/10/19 2:02 AM, Peter Krempa wrote:
> On Tue, Jul 09, 2019 at 12:46:30 -0500, Eric Blake wrote:
>> Even though we don't accept any flags, it is unfriendly to callers
>> that use the modern API to have to fall back to the flag-free API.
>>
>> Signed-off-by: Eric Blake <eblake(a)redhat.com>
>> ---
>> -vboxDomainSave(virDomainPtr dom, const char *path
ATTRIBUTE_UNUSED)
>> +vboxDomainSaveFlags(virDomainPtr dom, const char *path ATTRIBUTE_UNUSED,
>> + const char *dxml, unsigned int flags)
>> {
>> vboxDriverPtr data = dom->conn->privateData;
>> IConsole *console = NULL;
>> @@ -564,6 +565,9 @@ vboxDomainSave(virDomainPtr dom, const char *path
ATTRIBUTE_UNUSED)
>> nsresult rc;
>> int ret = -1;
>>
>> + virCheckFlags(0, -1);
>> + virCheckNonNullArgReturn(dxml, -1);
>
> This reports: invalid argument: dxml in vboxDomainSave must not be NULL
>
Revisiting this thread:
internal.h has:
virCheckNullArgGoto (complains if argument is not NULL)
virCheckNonNullArgReturn (complains if argument is NULL)
virCheckNonNullArgGoto (complains if argument is NULL)
but is missing virCheckNullArgReturn, which is the form I really meant
to use here. I have no idea if that missing macro is intentional, but it
would be easy enough to add.
> I'm not certain that the internal function name makes sense
to external
> users.
In which case I can hand-roll a more specific error instead of reusing
the common macro. But I do see pre-existing uses of
virCheckNonNullArgXXX in src/esx and src/vz prior to this patch, so it's
not the first time we've used the common macros.
Directly calling virReportInvalidNonNullArg() would be the only way to
hand-roll this properly, but no one outside of internal.h and
src/util/virobject.c uses it. I'd prefer to sick with
virCheckNullArgReturn().
>> +static int
>> +vboxDomainSave(virDomainPtr dom, const char *path)
>> +{
>> + return vboxDomainSaveFlags(dom, path, NULL, 0);
>
> So, this passes NULL 'dxml' into vboxDomainSaveFlags which explicitly
> rejects it. What's the point?
>
> Ah I see. Was the above supposed to be virCheckNullArgGoto?
D'oh. Yes, I got it backwards. The function wants to succeed only if
the user omitted the optional dxml argument.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
2:09 a.m.
New subject: [libvirt] [PATCH v2 1/9] vbox: Add various vir*Flags API
On Tue, Jul 09, 2019 at 12:46:30 -0500, Eric Blake wrote:
Even though we don't accept any flags, it is unfriendly to
callers
that use the modern API to have to fall back to the flag-free API.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/vbox/vbox_common.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
[...]
@@ -2775,6 +2788,11 @@ static int vboxDomainSetMemory(virDomainPtr
dom, unsigned long memory)
return ret;
}
+static int vboxDomainSetMemory(virDomainPtr dom, unsigned long memory)
+{
+ return vboxDomainSetMemoryFlags(dom, memory, 0);
The old API was operating on a live VM only so this shim should imply
VIR_DOMAIN_AFFECT_LIVE.
+}
+
static int vboxDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info)
{
vboxDriverPtr data = dom->conn->privateData;
7:13 a.m.
New subject: [libvirt] [PATCH v2 1/9] vbox: Add various vir*Flags API
On 7/10/19 2:09 AM, Peter Krempa wrote:
On Tue, Jul 09, 2019 at 12:46:30 -0500, Eric Blake wrote:
> Even though we don't accept any flags, it is unfriendly to callers
> that use the modern API to have to fall back to the flag-free API.
>
> Signed-off-by: Eric Blake <eblake(a)redhat.com>
> ---
> src/vbox/vbox_common.c | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
[...]
> @@ -2775,6 +2788,11 @@ static int vboxDomainSetMemory(virDomainPtr dom, unsigned long
memory)
> return ret;
> }
>
> +static int vboxDomainSetMemory(virDomainPtr dom, unsigned long memory)
> +{
> + return vboxDomainSetMemoryFlags(dom, memory, 0);
The old API was operating on a live VM only so this shim should imply
VIR_DOMAIN_AFFECT_LIVE.
Hmm. Commit 667ac11e used flag 0 for the test driver. But if I want to
reduce the number of driver callbacks by instead teaching the remote
driver when to call the legacy RPC, it's easier if ALL drivers shim-call
SetMemoryFlags(VIR_DOMAIN_AFFECT_LIVE) rather than some calling with 0
and others calling with AFFECT_LIVE.
The current list:
v5.5.0:src/hyperv/hyperv_driver.c: return
hypervDomainSetMemoryFlags(domain, memory, 0);
v5.5.0:src/libxl/libxl_driver.c: return
libxlDomainSetMemoryFlags(dom, memory, VIR_DOMAIN_MEM_LIVE);
v5.5.0:src/libxl/libxl_driver.c: return
libxlDomainSetMemoryFlags(dom, memory, VIR_DOMAIN_MEM_MAXIMUM);
v5.5.0:src/lxc/lxc_driver.c: return lxcDomainSetMemoryFlags(dom,
newmem, VIR_DOMAIN_AFFECT_LIVE);
v5.5.0:src/lxc/lxc_driver.c: return lxcDomainSetMemoryFlags(dom,
newmax, VIR_DOMAIN_MEM_MAXIMUM);
v5.5.0:src/qemu/qemu_driver.c: return qemuDomainSetMemoryFlags(dom,
newmem, VIR_DOMAIN_AFFECT_LIVE);
v5.5.0:src/qemu/qemu_driver.c: return qemuDomainSetMemoryFlags(dom,
memory, VIR_DOMAIN_MEM_MAXIMUM);
my series adds:
test
vbox
xenapi
So it looks like hyperv has a bug in being inconsistent from everyone
else, if I follow your advice for the 3 new shims, although it currently
lacks a SetMaxMemory API at all.
You also appear to be right that making SetMaxMemory a shim everywhere
for SetMemoryFlags(VIR_DOMAIN_MEM_MAXMIUM) is a sensible idea, at least
for drivers that have a notion of setting max memory, but since not all
drivers had SetMaxMemory, it's harder to argue that the presence of the
new interface must require the legacy pair (especially if the new
interface blindly requires a specific flags pattern). I'll have to look
more closely at what each driver is doing.
Our public docs currently state:
* and will fail for transient domains. If neither flag is specified
* (that is, @flags is VIR_DOMAIN_AFFECT_CURRENT), then an inactive domain
* modifies persistent setup, while an active domain is hypervisor-dependent
* on whether just live or both live and persistent state is changed.
which means that calling SetMemory() may be a synonym to either
SetMemoryFlags(_CURRENT==0) or to SetMemoryFlags(_LIVE). But by
tweaking hyperv, we could change it so that the spec is more
tightly-worded as always being equivalent to SetMemoryFlags(_LIVE).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
12:46 p.m.
New subject: [libvirt] [PATCH v2 2/9] xenapi: Add various vir*Flags API
Even though we don't accept any flags, it is unfriendly to callers
that use the modern API to have to fall back to the flag-free API.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/xenapi/xenapi_driver.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index 672117822f..026acef680 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -1184,19 +1184,21 @@ xenapiDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus)
}
/*
- * xenapiDomainPinVcpu
+ * xenapiDomainPinVcpuFlags
*
* Dynamically change the real CPUs which can be allocated to a virtual CPU
* Returns 0 on success or -1 in case of error
*/
static int
-xenapiDomainPinVcpu(virDomainPtr dom, unsigned int vcpu ATTRIBUTE_UNUSED,
- unsigned char *cpumap, int maplen)
+xenapiDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu ATTRIBUTE_UNUSED,
+ unsigned char *cpumap, int maplen,
+ unsigned int flags)
{
char *value = NULL;
xen_vm vm;
xen_vm_set *vms;
xen_session *session = ((struct _xenapiPrivate
*)(dom->conn->privateData))->session;
+ virCheckFlags(0, -1);
if (xen_vm_get_by_name_label(session, &vms, dom->name) && vms->size
> 0) {
if (vms->size != 1) {
xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR,
@@ -1225,6 +1227,19 @@ xenapiDomainPinVcpu(virDomainPtr dom, unsigned int vcpu
ATTRIBUTE_UNUSED,
return -1;
}
+/*
+ * xenapiDomainPinVcpu
+ *
+ * Dynamically change the real CPUs which can be allocated to a virtual CPU
+ * Returns 0 on success or -1 in case of error
+ */
+static int
+xenapiDomainPinVcpu(virDomainPtr dom, unsigned int vcpu,
+ unsigned char *cpumap, int maplen)
+{
+ return xenapiDomainPinVcpuFlags(dom, vcpu, cpumap, maplen, 0);
+}
+
/*
* xenapiDomainGetVcpus
*
@@ -2029,6 +2044,7 @@ static virHypervisorDriver xenapiHypervisorDriver = {
.domainSetVcpusFlags = xenapiDomainSetVcpusFlags, /* 0.8.5 */
.domainGetVcpusFlags = xenapiDomainGetVcpusFlags, /* 0.8.5 */
.domainPinVcpu = xenapiDomainPinVcpu, /* 0.8.0 */
+ .domainPinVcpuFlags = xenapiDomainPinVcpuFlags, /* 5.6.0 */
.domainGetVcpus = xenapiDomainGetVcpus, /* 0.8.0 */
.domainGetMaxVcpus = xenapiDomainGetMaxVcpus, /* 0.8.0 */
.domainGetXMLDesc = xenapiDomainGetXMLDesc, /* 0.8.0 */
--
2.20.1
2:04 a.m.
New subject: [libvirt] [PATCH v2 2/9] xenapi: Add various vir*Flags API
On Tue, Jul 09, 2019 at 12:46:31 -0500, Eric Blake wrote:
Even though we don't accept any flags, it is unfriendly to
callers
that use the modern API to have to fall back to the flag-free API.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/xenapi/xenapi_driver.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
ACK
2:08 a.m.
New subject: [libvirt] [PATCH v2 2/9] xenapi: Add various vir*Flags API
On Tue, Jul 09, 2019 at 12:46:31 -0500, Eric Blake wrote:
Even though we don't accept any flags, it is unfriendly to
callers
that use the modern API to have to fall back to the flag-free API.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/xenapi/xenapi_driver.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
[...]
@@ -1225,6 +1227,19 @@ xenapiDomainPinVcpu(virDomainPtr dom, unsigned
int vcpu ATTRIBUTE_UNUSED,
return -1;
}
+/*
+ * xenapiDomainPinVcpu
+ *
+ * Dynamically change the real CPUs which can be allocated to a virtual CPU
+ * Returns 0 on success or -1 in case of error
+ */
+static int
+xenapiDomainPinVcpu(virDomainPtr dom, unsigned int vcpu,
+ unsigned char *cpumap, int maplen)
+{
+ return xenapiDomainPinVcpuFlags(dom, vcpu, cpumap, maplen, 0);
Actually in the qemu driver we pass in VIR_DOMAIN_AFFECT_LIVE as the
flags here, because the old API worked only on the live definition.
12:46 p.m.
New subject: [libvirt] [PATCH v2 3/9] maint: Enhance check-driverimpls.pl to check for API pairing
As shown in recent patches, several drivers provided only an older
counterpart of an API, making it harder to uniformly use the newer
preferred API form. We can prevent future instances of this by
enhancing 'make syntax-check' to flag any time a modern API is
forgotten when an older API is present. It also flags if a modern API
is provided without an old counterpart; but thankfully, that situation
didn't flag, which gives us some room for future patches to confine
the magic of API pairs to just src/libvirt*.c and the remote driver.
Also, drop support for special-casing xenUnified, since 1dac5fbbbb0
dropped support for that naming scheme.
---
src/check-driverimpls.pl | 33 +++++++++++++++++++++++++++++++--
1 file changed, 31 insertions(+), 2 deletions(-)
diff --git a/src/check-driverimpls.pl b/src/check-driverimpls.pl
index b175e710f1..34273ddbba 100755
--- a/src/check-driverimpls.pl
+++ b/src/check-driverimpls.pl
@@ -23,14 +23,41 @@ use warnings;
my $intable = 0;
my $table;
my $mainprefix;
+my %apis;
+
+# API pairs where a driver should provide both or neither alternative.
+my %pairs = (
+ 'domainShutdown' => 'domainShutdownFlags',
+ 'domainDestroy' => 'domainDestroyFlags',
+ 'domainSetMemory' => 'domainSetMemoryFlags',
+ 'domainSave' => 'domainSaveFlags',
+ 'domainRestore' => 'domainRestoreFlags',
+ 'domainSetVcpus' => 'domainSetVcpusFlags',
+ 'domainPinVcpu' => 'domainPinVcpuFlags',
+ 'domainCreate' => 'domainCreateWithFlags',
+ 'domainDefineXML' => 'domainDefineXMLFlags',
+ 'domainUndefine' => 'domainUndefineFlags',
+ 'domainAttachDevice' => 'domainAttachDeviceFlags',
+ 'domainDetachDevice' => 'domainDetachDeviceFlags',
+ 'domainGetSchedulerParameters' =>
'domainGetSchedulerParametersFlags',
+ 'domainSetSchedulerParameters' =>
'domainSetSchedulerParametersFlags',
+ 'nodeDeviceDettach' => 'nodeDeviceDetachFlags',
+);
my $status = 0;
while (<>) {
if ($intable) {
if (/}/) {
+ while (my ($old, $new) = each %pairs) {
+ if (exists $apis{$old} != exists $apis{$new}) {
+ print "$ARGV:$. Inconsistent paired API '$old' vs.
'$new'\n";
+ $status = 1;
+ }
+ }
$intable = 0;
$table = undef;
$mainprefix = undef;
+ %apis = ();
} elsif (/\.(\w+)\s*=\s*(\w+),?/) {
my $api = $1;
my $impl = $2;
@@ -39,9 +66,11 @@ while (<>) {
next if $api eq "name";
next if $impl eq "NULL";
+ $apis{$api} = 1;
+
my $suffix = $impl;
my $prefix = $impl;
- $prefix =~ s/^([a-z]+(?:Unified)?)(.*?)$/$1/;
+ $prefix =~ s/^([a-z]+)(.*?)$/$1/;
if (defined $mainprefix) {
if ($mainprefix ne $prefix) {
@@ -53,7 +82,7 @@ while (<>) {
}
if ($api !~ /^$mainprefix/) {
- $suffix =~ s/^[a-z]+(?:Unified)?//;
+ $suffix =~ s/^[a-z]+//;
$suffix =~ s/^([A-Z]+)/lc $1/e;
}
--
2.20.1
2:16 a.m.
New subject: [libvirt] [PATCH v2 3/9] maint: Enhance check-driverimpls.pl to check for API pairing
On Tue, Jul 09, 2019 at 12:46:32 -0500, Eric Blake wrote:
As shown in recent patches, several drivers provided only an older
counterpart of an API, making it harder to uniformly use the newer
preferred API form. We can prevent future instances of this by
enhancing 'make syntax-check' to flag any time a modern API is
forgotten when an older API is present. It also flags if a modern API
is provided without an old counterpart; but thankfully, that situation
didn't flag, which gives us some room for future patches to confine
the magic of API pairs to just src/libvirt*.c and the remote driver.
Also, drop support for special-casing xenUnified, since 1dac5fbbbb0
dropped support for that naming scheme.
Please put it in a separate patch.
---
src/check-driverimpls.pl | 33 +++++++++++++++++++++++++++++++--
1 file changed, 31 insertions(+), 2 deletions(-)
diff --git a/src/check-driverimpls.pl b/src/check-driverimpls.pl
index b175e710f1..34273ddbba 100755
--- a/src/check-driverimpls.pl
+++ b/src/check-driverimpls.pl
@@ -23,14 +23,41 @@ use warnings;
my $intable = 0;
my $table;
my $mainprefix;
+my %apis;
+
+# API pairs where a driver should provide both or neither alternative.
+my %pairs = (
+ 'domainShutdown' => 'domainShutdownFlags',
+ 'domainDestroy' => 'domainDestroyFlags',
+ 'domainSetMemory' => 'domainSetMemoryFlags',
domainSetMaxMemory is in the same relationship here as it just implies
VIR_DOMAIN_MEM_MAXIMUM passed to the *Flags API.
+ 'domainSave' => 'domainSaveFlags',
+ 'domainRestore' => 'domainRestoreFlags',
+ 'domainSetVcpus' => 'domainSetVcpusFlags',
+ 'domainPinVcpu' => 'domainPinVcpuFlags',
+ 'domainCreate' => 'domainCreateWithFlags',
+ 'domainDefineXML' => 'domainDefineXMLFlags',
+ 'domainUndefine' => 'domainUndefineFlags',
+ 'domainAttachDevice' => 'domainAttachDeviceFlags',
+ 'domainDetachDevice' => 'domainDetachDeviceFlags',
+ 'domainGetSchedulerParameters' =>
'domainGetSchedulerParametersFlags',
+ 'domainSetSchedulerParameters' =>
'domainSetSchedulerParametersFlags',
+ 'nodeDeviceDettach' => 'nodeDeviceDetachFlags',
+);
my $status = 0;
while (<>) {
if ($intable) {
if (/}/) {
+ while (my ($old, $new) = each %pairs) {
+ if (exists $apis{$old} != exists $apis{$new}) {
+ print "$ARGV:$. Inconsistent paired API '$old' vs.
'$new'\n";
Without the context of the patch the message does not seem to explain
what's happening to the unindoctrinated. I don't have a better
suggestion though.
+ $status = 1;
+ }
+ }
$intable = 0;
$table = undef;
$mainprefix = undef;
ACK if you split out the "Also fix this" stuff. Your call whether
domainSetMaxMemory is worth covering or the error message needs
changing.
12:46 p.m.
New subject: [libvirt] [PATCH v2 4/9] bhyve: Ignore no-op flag during virDomainUndefine
Copy what esx does in ignoring the SNAPSHOTS_METADATA flag as a no-op,
and in line with the recent doc tweak in commit c049f022.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/bhyve/bhyve_driver.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 4ce9ef0b95..84720e89e7 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -584,7 +584,11 @@ bhyveDomainUndefineFlags(virDomainPtr domain, unsigned int flags)
virDomainObjPtr vm;
int ret = -1;
- virCheckFlags(0, -1);
+ /* No managed save, so we explicitly reject
+ * VIR_DOMAIN_UNDEFINE_MANAGED_SAVE. No snapshot metadata for
+ * bhyve, so we can trivially ignore that flag. */
+ virCheckFlags(VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1);
+
if (!(vm = bhyveDomObjFromDomain(domain)))
goto cleanup;
--
2.20.1
2:18 a.m.
New subject: [libvirt] [PATCH v2 4/9] bhyve: Ignore no-op flag during virDomainUndefine
On Tue, Jul 09, 2019 at 12:46:33 -0500, Eric Blake wrote:
Copy what esx does in ignoring the SNAPSHOTS_METADATA flag as a
no-op,
and in line with the recent doc tweak in commit c049f022.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
ACK
12:46 p.m.
New subject: [libvirt] [PATCH v2 5/9] libxl: Ignore no-op flag during virDomainUndefine
Copy what esx does in ignoring the SNAPSHOTS_METADATA flag as a no-op,
and in line with the recent doc tweak in commit c049f022.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/libxl/libxl_driver.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 731700ded6..89959890c3 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2921,7 +2921,9 @@ libxlDomainUndefineFlags(virDomainPtr dom,
char *name = NULL;
int ret = -1;
- virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE, -1);
+ /* No snapshot support, so we can trivially ignore that flag. */
+ virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE |
+ VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1);
if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
--
2.20.1
2:19 a.m.
New subject: [libvirt] [PATCH v2 5/9] libxl: Ignore no-op flag during virDomainUndefine
On Tue, Jul 09, 2019 at 12:46:34 -0500, Eric Blake wrote:
Copy what esx does in ignoring the SNAPSHOTS_METADATA flag as a
no-op,
and in line with the recent doc tweak in commit c049f022.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
ACK
12:46 p.m.
New subject: [libvirt] [PATCH v2 6/9] lxc: Ignore no-op flag during virDomainUndefine
Copy what esx does in ignoring the SNAPSHOTS_METADATA flag as a no-op,
and in line with the recent doc tweak in commit c049f022.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/lxc/lxc_driver.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 9db2a02dee..a1bb29d5ae 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -491,7 +491,10 @@ static int lxcDomainUndefineFlags(virDomainPtr dom,
int ret = -1;
virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
- virCheckFlags(0, -1);
+ /* No managed save, so we explicitly reject
+ * VIR_DOMAIN_UNDEFINE_MANAGED_SAVE. No snapshot metadata for
+ * lxc, so we can trivially ignore that flag. */
+ virCheckFlags(VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1);
if (!(vm = lxcDomObjFromDomain(dom)))
goto cleanup;
--
2.20.1
2:19 a.m.
New subject: [libvirt] [PATCH v2 6/9] lxc: Ignore no-op flag during virDomainUndefine
On Tue, Jul 09, 2019 at 12:46:35 -0500, Eric Blake wrote:
Copy what esx does in ignoring the SNAPSHOTS_METADATA flag as a
no-op,
and in line with the recent doc tweak in commit c049f022.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
ACK
12:46 p.m.
New subject: [libvirt] [PATCH v2 7/9] openvz: Ignore no-op flag during virDomainUndefine
Copy what esx does in ignoring the SNAPSHOTS_METADATA flag as a no-op,
and in line with the recent doc tweak in commit c049f022.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/openvz/openvz_driver.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 39eeb2c12e..19c7db48ed 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -1126,7 +1126,10 @@ openvzDomainUndefineFlags(virDomainPtr dom,
int ret = -1;
int status;
- virCheckFlags(0, -1);
+ /* No managed save, so we explicitly reject
+ * VIR_DOMAIN_UNDEFINE_MANAGED_SAVE. No snapshot metadata for
+ * openvz, so we can trivially ignore that flag. */
+ virCheckFlags(VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1);
openvzDriverLock(driver);
if (!(vm = openvzDomObjFromDomainLocked(driver, dom->uuid)))
--
2.20.1
2:20 a.m.
New subject: [libvirt] [PATCH v2 7/9] openvz: Ignore no-op flag during virDomainUndefine
On Tue, Jul 09, 2019 at 12:46:36 -0500, Eric Blake wrote:
Copy what esx does in ignoring the SNAPSHOTS_METADATA flag as a
no-op,
and in line with the recent doc tweak in commit c049f022.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
ACK
12:46 p.m.
New subject: [libvirt] [PATCH v2 8/9] vmware: Ignore no-op flag during virDomainUndefine
Copy what esx does in ignoring the SNAPSHOTS_METADATA flag as a no-op,
and in line with the recent doc tweak in commit c049f022.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/vmware/vmware_driver.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
index 1bc8a06c39..934378808d 100644
--- a/src/vmware/vmware_driver.c
+++ b/src/vmware/vmware_driver.c
@@ -776,7 +776,10 @@ vmwareDomainUndefineFlags(virDomainPtr dom,
virDomainObjPtr vm;
int ret = -1;
- virCheckFlags(0, -1);
+ /* No managed save, so we explicitly reject
+ * VIR_DOMAIN_UNDEFINE_MANAGED_SAVE. No snapshot metadata for
+ * vmware, so we can trivially ignore that flag. */
+ virCheckFlags(VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1);
vmwareDriverLock(driver);
if (!(vm = vmwareDomObjFromDomainLocked(driver, dom->uuid)))
--
2.20.1
2:20 a.m.
New subject: [libvirt] [PATCH v2 8/9] vmware: Ignore no-op flag during virDomainUndefine
On Tue, Jul 09, 2019 at 12:46:37 -0500, Eric Blake wrote:
Copy what esx does in ignoring the SNAPSHOTS_METADATA flag as a
no-op,
and in line with the recent doc tweak in commit c049f022.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
ACK
12:46 p.m.
New subject: [libvirt] [PATCH v2 9/9] xenapi: Ignore no-op flag during virDomainUndefine
Copy what esx does in ignoring the SNAPSHOTS_METADATA flag as a no-op,
and in line with the recent doc tweak in commit c049f022.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/xenapi/xenapi_driver.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index 026acef680..2cc6dbd22d 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -1791,7 +1791,11 @@ xenapiDomainUndefineFlags(virDomainPtr dom, unsigned int flags)
struct xen_vm_set *vms;
xen_vm vm;
xen_session *session = ((struct _xenapiPrivate
*)(dom->conn->privateData))->session;
- virCheckFlags(0, -1);
+
+ /* No managed save, so we explicitly reject
+ * VIR_DOMAIN_UNDEFINE_MANAGED_SAVE. No snapshot metadata for
+ * xenapi, so we can trivially ignore that flag. */
+ virCheckFlags(VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1);
if (xen_vm_get_by_name_label(session, &vms, dom->name) && vms->size
> 0) {
if (vms->size != 1) {
--
2.20.1
2:21 a.m.
New subject: [libvirt] [PATCH v2 9/9] xenapi: Ignore no-op flag during virDomainUndefine
On Tue, Jul 09, 2019 at 12:46:38 -0500, Eric Blake wrote:
Copy what esx does in ignoring the SNAPSHOTS_METADATA flag as a
no-op,
and in line with the recent doc tweak in commit c049f022.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
ACK
1921
days inactive
1964
days old
23 comments
2 participants
participants (2)
-
Eric Blake -
Peter Krempa