9:38 a.m.
Two bugs has been filed about wrong usage of api flags and virsh commands.
After investigation of the core issue I've decide to introduce same helper
macros as we have in virsh code to check mutually exclusive flags.
This patch series also includes the bug fixes and some other cleanups in
setvcpus code.
Luyao Huang (2):
qemu: check if domain is really active when do setvcpus with --live
tools: fix the wrong check when use virsh setvcpus --maximum
Pavel Hrdina (4):
internal: introduce macro helpers to reject exclusive flags
use new macro helpers to check exclusive flags
qemu: cleanup setvcpus
qemu: fix set vcpus on host without NUMA
src/internal.h | 80 +++++++++++
src/libvirt-domain-snapshot.c | 45 ++----
src/libvirt-domain.c | 288 +++++++++++--------------------------
src/qemu/qemu_driver.c | 42 +++---
src/storage/storage_backend_disk.c | 10 +-
src/storage/storage_backend_fs.c | 11 +-
tools/virsh-domain.c | 30 +---
7 files changed, 208 insertions(+), 298 deletions(-)
--
2.0.5
9:38 a.m.
New subject: [libvirt] [PATCH 1/6] internal: introduce macro helpers to reject exclusive flags
Inspired by commit 7e437ee7 that introduced similar macros for virsh
commands so we don't have to repeat the same code all over.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/internal.h | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 80 insertions(+)
diff --git a/src/internal.h b/src/internal.h
index 4d473af..5885f03 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -327,6 +327,86 @@
} \
} while (0)
+/* Macros to help dealing with mutually exclusive flags. */
+
+/**
+ * VIR_EXCLUSIVE_FLAGS_EXPR_RET:
+ *
+ * @NAME1: String containing the name of the flag.
+ * @EXPR1: Expression to validate the flag.
+ * @NAME2: String containing the name of the flag.
+ * @EXPR2: Expression to validate the flag.
+ *
+ * Reject mutually exclusive API flags. Use the provided expression
+ * to check the flags.
+ *
+ * This helper does an early return and therefore it has to be called
+ * before anything that would require cleanup.
+ */
+# define VIR_EXCLUSIVE_FLAGS_EXPR_RET(NAME1, EXPR1, NAME2, EXPR2) \
+ if ((EXPR1) && (EXPR2)) { \
+ virReportInvalidArg(ctl, \
+ _("Flags '%s' and '%s' are mutually
exclusive"),\
+ NAME1, NAME2); \
+ return -1; \
+ }
+
+/**
+ * VIR_EXCLUSIVE_FLAGS_VAR_RET:
+ *
+ * @FLAG1: First flag to be checked
+ * @FLAG2: Second flag to be checked
+ *
+ * Reject mutually exclusive API flags. The checked flags are compared
+ * with flags variable.
+ *
+ * This helper does an early return and therefore it has to be called
+ * before anything that would require cleanup.
+ */
+# define VIR_EXCLUSIVE_FLAGS_RET(FLAG1, FLAG2) \
+ VIR_EXCLUSIVE_FLAGS_EXPR_RET(#FLAG1, flags & FLAG1, #FLAG2, flags & FLAG2)
+
+/**
+ * VIR_EXCLUSIVE_FLAGS_EXPR_GOTO:
+ *
+ * @NAME1: String containing the name of the flag.
+ * @EXPR1: Expression to validate the flag.
+ * @NAME2: String containing the name of the flag.
+ * @EXPR2: Expression to validate the flag.
+ * @LABEL: label to jump to.
+ *
+ * Reject mutually exclusive API flags. Use the provided expression
+ * to check the flag.
+ *
+ * Returns nothing. Jumps to a label if unsupported flags were
+ * passed to it.
+ */
+# define VIR_EXCLUSIVE_FLAGS_EXPR_GOTO(NAME1, EXPR1, NAME2, EXPR2, LABEL) \
+ if ((EXPR1) && (EXPR2)) { \
+ virReportInvalidArg(ctl, \
+ _("Flags '%s' and '%s' are mutually
exclusive"),\
+ NAME1, NAME2); \
+ goto LABEL; \
+ }
+
+/**
+ * VIR_EXCLUSIVE_FLAGS_VAR_GOTO:
+ *
+ * @FLAG1: First flag to be checked.
+ * @FLAG2: Second flag to be checked.
+ * @LABEL: label to jump to.
+ *
+ * Reject mutually exclusive API flags. The checked flags are compared
+ * with flags variable.
+ *
+ * Returns nothing. Jumps to a label if unsupported flags were
+ * passed to it.
+ */
+# define VIR_EXCLUSIVE_FLAGS_GOTO(FLAG1, FLAG2, LABEL) \
+ VIR_EXCLUSIVE_FLAGS_EXPR_GOTO(#FLAG1, flags & FLAG1, \
+ #FLAG2, flags & FLAG2, LABEL)
+
+
# define virCheckNonNullArgReturn(argname, retval) \
do { \
if (argname == NULL) { \
--
2.0.5
9:58 a.m.
New subject: [libvirt] [PATCH 7/6] qemu: move virDomainLiveConfigHelperMethod right after BeginJob
We should call virDomainLiveConfigHelperMethod ASAP because this
function transfers VIR_DOMAIN_AFFECT_CURRENT to VIR_DOMAIN_AFFECT_LIVE
or VIR_DOMAIN_AFFECT_CONFIG. All other additional checks for those two
flags should consider that the user give us VIR_DOMAIN_AFFECT_CURRENT.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_driver.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index eb86d68..575d3b2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4881,6 +4881,10 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
+ if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags,
+ &persistentDef) < 0)
+ goto endjob;
+
if (flags & VIR_DOMAIN_AFFECT_LIVE) {
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -4905,10 +4909,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
goto endjob;
}
- if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags,
- &persistentDef) < 0)
- goto endjob;
-
/* MAXIMUM cannot be mixed with LIVE. */
if ((flags & VIR_DOMAIN_VCPU_MAXIMUM) && (flags &
VIR_DOMAIN_AFFECT_LIVE)) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
--
2.0.5
11:13 a.m.
New subject: [libvirt] [PATCH 7/6] qemu: move virDomainLiveConfigHelperMethod right after BeginJob
On Fri, Mar 20, 2015 at 15:58:56 +0100, Pavel Hrdina wrote:
We should call virDomainLiveConfigHelperMethod ASAP because this
function transfers VIR_DOMAIN_AFFECT_CURRENT to VIR_DOMAIN_AFFECT_LIVE
or VIR_DOMAIN_AFFECT_CONFIG. All other additional checks for those two
flags should consider that the user give us VIR_DOMAIN_AFFECT_CURRENT.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_driver.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
ACK, although this should be 0.5/6 or 1.5/6. Basically just push this
first.
Peter
11:57 a.m.
New subject: [libvirt] [PATCH 7/6] qemu: move virDomainLiveConfigHelperMethod right after BeginJob
On Tue, Mar 24, 2015 at 05:13:07PM +0100, Peter Krempa wrote:
On Fri, Mar 20, 2015 at 15:58:56 +0100, Pavel Hrdina wrote:
> We should call virDomainLiveConfigHelperMethod ASAP because this
> function transfers VIR_DOMAIN_AFFECT_CURRENT to VIR_DOMAIN_AFFECT_LIVE
> or VIR_DOMAIN_AFFECT_CONFIG. All other additional checks for those two
> flags should consider that the user give us VIR_DOMAIN_AFFECT_CURRENT.
>
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
ACK, although this should be 0.5/6 or 1.5/6. Basically just push this
first.
Peter
Thanks for review, I'll push it shortly.
Pavel
10:40 a.m.
New subject: [libvirt] [PATCH 1/6] internal: introduce macro helpers to reject exclusive flags
On Fri, Mar 20, 2015 at 15:38:59 +0100, Pavel Hrdina wrote:
Inspired by commit 7e437ee7 that introduced similar macros for virsh
commands so we don't have to repeat the same code all over.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/internal.h | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 80 insertions(+)
diff --git a/src/internal.h b/src/internal.h
index 4d473af..5885f03 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -327,6 +327,86 @@
} \
} while (0)
+/* Macros to help dealing with mutually exclusive flags. */
+
+/**
+ * VIR_EXCLUSIVE_FLAGS_EXPR_RET:
+ *
+ * @NAME1: String containing the name of the flag.
+ * @EXPR1: Expression to validate the flag.
+ * @NAME2: String containing the name of the flag.
+ * @EXPR2: Expression to validate the flag.
+ *
+ * Reject mutually exclusive API flags. Use the provided expression
+ * to check the flags.
+ *
+ * This helper does an early return and therefore it has to be called
+ * before anything that would require cleanup.
+ */
+# define VIR_EXCLUSIVE_FLAGS_EXPR_RET(NAME1, EXPR1, NAME2, EXPR2) \
+ if ((EXPR1) && (EXPR2)) { \
+ virReportInvalidArg(ctl, \
+ _("Flags '%s' and '%s' are mutually
exclusive"),\
+ NAME1, NAME2); \
+ return -1; \
+ }
+
While in virsh the above variant makes sense, because in some cases the
variables have different names than the corresponding flags, in case of
this code having it doesn't make sense.
+/**
+ * VIR_EXCLUSIVE_FLAGS_VAR_RET:
+ *
+ * @FLAG1: First flag to be checked
+ * @FLAG2: Second flag to be checked
A third argument @RET should be added here so that this can be used in
places that also return NULL or any other possible variable.
In virsh the assumption is that the flags are checked at first and thus
returning false makes sense, as every virsh command does that.
+ *
+ * Reject mutually exclusive API flags. The checked flags are compared
+ * with flags variable.
+ *
+ * This helper does an early return and therefore it has to be called
+ * before anything that would require cleanup.
+ */
+# define VIR_EXCLUSIVE_FLAGS_RET(FLAG1, FLAG2) \
+ VIR_EXCLUSIVE_FLAGS_EXPR_RET(#FLAG1, flags & FLAG1, #FLAG2, flags & FLAG2)
+
+/**
+ * VIR_EXCLUSIVE_FLAGS_EXPR_GOTO:
+ *
+ * @NAME1: String containing the name of the flag.
+ * @EXPR1: Expression to validate the flag.
+ * @NAME2: String containing the name of the flag.
+ * @EXPR2: Expression to validate the flag.
+ * @LABEL: label to jump to.
+ *
+ * Reject mutually exclusive API flags. Use the provided expression
+ * to check the flag.
+ *
+ * Returns nothing. Jumps to a label if unsupported flags were
+ * passed to it.
+ */
+# define VIR_EXCLUSIVE_FLAGS_EXPR_GOTO(NAME1, EXPR1, NAME2, EXPR2, LABEL) \
+ if ((EXPR1) && (EXPR2)) { \
+ virReportInvalidArg(ctl, \
+ _("Flags '%s' and '%s' are mutually
exclusive"),\
+ NAME1, NAME2); \
+ goto LABEL; \
+ }
+
Again, this function does not make sense in the library code.
+/**
+ * VIR_EXCLUSIVE_FLAGS_VAR_GOTO:
+ *
+ * @FLAG1: First flag to be checked.
+ * @FLAG2: Second flag to be checked.
+ * @LABEL: label to jump to.
+ *
+ * Reject mutually exclusive API flags. The checked flags are compared
+ * with flags variable.
+ *
+ * Returns nothing. Jumps to a label if unsupported flags were
+ * passed to it.
+ */
+# define VIR_EXCLUSIVE_FLAGS_GOTO(FLAG1, FLAG2, LABEL) \
+ VIR_EXCLUSIVE_FLAGS_EXPR_GOTO(#FLAG1, flags & FLAG1, \
+ #FLAG2, flags & FLAG2, LABEL)
+
+
# define virCheckNonNullArgReturn(argname, retval) \
do { \
if (argname == NULL) { \
While this is okay.
Peter
noon
New subject: [libvirt] [PATCH 1/6] internal: introduce macro helpers to reject exclusive flags
On Tue, Mar 24, 2015 at 04:40:17PM +0100, Peter Krempa wrote:
On Fri, Mar 20, 2015 at 15:38:59 +0100, Pavel Hrdina wrote:
> Inspired by commit 7e437ee7 that introduced similar macros for virsh
> commands so we don't have to repeat the same code all over.
>
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
> src/internal.h | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 80 insertions(+)
>
> diff --git a/src/internal.h b/src/internal.h
> index 4d473af..5885f03 100644
> --- a/src/internal.h
> +++ b/src/internal.h
> @@ -327,6 +327,86 @@
> } \
> } while (0)
>
> +/* Macros to help dealing with mutually exclusive flags. */
> +
> +/**
> + * VIR_EXCLUSIVE_FLAGS_EXPR_RET:
> + *
> + * @NAME1: String containing the name of the flag.
> + * @EXPR1: Expression to validate the flag.
> + * @NAME2: String containing the name of the flag.
> + * @EXPR2: Expression to validate the flag.
> + *
> + * Reject mutually exclusive API flags. Use the provided expression
> + * to check the flags.
> + *
> + * This helper does an early return and therefore it has to be called
> + * before anything that would require cleanup.
> + */
> +# define VIR_EXCLUSIVE_FLAGS_EXPR_RET(NAME1, EXPR1, NAME2, EXPR2) \
> + if ((EXPR1) && (EXPR2)) {
\
> + virReportInvalidArg(ctl, \
> + _("Flags '%s' and '%s' are
mutually exclusive"),\
> + NAME1, NAME2); \
> + return -1; \
> + }
> +
While in virsh the above variant makes sense, because in some cases the
variables have different names than the corresponding flags, in case of
this code having it doesn't make sense.
You're right. I've only used the other macros.
> +/**
> + * VIR_EXCLUSIVE_FLAGS_VAR_RET:
> + *
> + * @FLAG1: First flag to be checked
> + * @FLAG2: Second flag to be checked
A third argument @RET should be added here so that this can be used in
places that also return NULL or any other possible variable.
In virsh the assumption is that the flags are checked at first and thus
returning false makes sense, as every virsh command does that.
I'll add the @RET argument.
> + *
> + * Reject mutually exclusive API flags. The checked flags are compared
> + * with flags variable.
> + *
> + * This helper does an early return and therefore it has to be called
> + * before anything that would require cleanup.
> + */
> +# define VIR_EXCLUSIVE_FLAGS_RET(FLAG1, FLAG2) \
> + VIR_EXCLUSIVE_FLAGS_EXPR_RET(#FLAG1, flags & FLAG1, #FLAG2, flags &
FLAG2)
> +
> +/**
> + * VIR_EXCLUSIVE_FLAGS_EXPR_GOTO:
> + *
> + * @NAME1: String containing the name of the flag.
> + * @EXPR1: Expression to validate the flag.
> + * @NAME2: String containing the name of the flag.
> + * @EXPR2: Expression to validate the flag.
> + * @LABEL: label to jump to.
> + *
> + * Reject mutually exclusive API flags. Use the provided expression
> + * to check the flag.
> + *
> + * Returns nothing. Jumps to a label if unsupported flags were
> + * passed to it.
> + */
> +# define VIR_EXCLUSIVE_FLAGS_EXPR_GOTO(NAME1, EXPR1, NAME2, EXPR2, LABEL) \
> + if ((EXPR1) && (EXPR2)) {
\
> + virReportInvalidArg(ctl, \
> + _("Flags '%s' and '%s' are
mutually exclusive"),\
> + NAME1, NAME2); \
> + goto LABEL; \
> + }
> +
Again, this function does not make sense in the library code.
> +/**
> + * VIR_EXCLUSIVE_FLAGS_VAR_GOTO:
> + *
> + * @FLAG1: First flag to be checked.
> + * @FLAG2: Second flag to be checked.
> + * @LABEL: label to jump to.
> + *
> + * Reject mutually exclusive API flags. The checked flags are compared
> + * with flags variable.
> + *
> + * Returns nothing. Jumps to a label if unsupported flags were
> + * passed to it.
> + */
> +# define VIR_EXCLUSIVE_FLAGS_GOTO(FLAG1, FLAG2, LABEL) \
> + VIR_EXCLUSIVE_FLAGS_EXPR_GOTO(#FLAG1, flags & FLAG1, \
> + #FLAG2, flags & FLAG2, LABEL)
> +
> +
> # define virCheckNonNullArgReturn(argname, retval) \
> do { \
> if (argname == NULL) { \
While this is okay.
Peter
Thanks for review, I'll send v2.
Pavel
9:39 a.m.
New subject: [libvirt] [PATCH 2/6] use new macro helpers to check exclusive flags
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/libvirt-domain-snapshot.c | 45 ++----
src/libvirt-domain.c | 288 +++++++++++--------------------------
src/qemu/qemu_driver.c | 9 +-
src/storage/storage_backend_disk.c | 10 +-
src/storage/storage_backend_fs.c | 11 +-
5 files changed, 106 insertions(+), 257 deletions(-)
diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c
index 9feb669..0d5c5e8 100644
--- a/src/libvirt-domain-snapshot.c
+++ b/src/libvirt-domain-snapshot.c
@@ -228,22 +228,13 @@ virDomainSnapshotCreateXML(virDomainPtr domain,
__FUNCTION__);
goto error;
}
- if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) &&
- (flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) {
- virReportInvalidArg(flags,
- _("'redefine' and 'no metadata' flags in
%s are "
- "mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
- if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) &&
- (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
- virReportInvalidArg(flags,
- _("'redefine' and 'halt' flags in %s are
mutually "
- "exclusive"),
- __FUNCTION__);
- goto error;
- }
+
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE,
+ VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA,
+ error);
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE,
+ VIR_DOMAIN_SNAPSHOT_CREATE_HALT,
+ error);
if (conn->driver->domainSnapshotCreateXML) {
virDomainSnapshotPtr ret;
@@ -1082,14 +1073,9 @@ virDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
virCheckReadOnlyGoto(conn->flags, error);
- if ((flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) &&
- (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
- virReportInvalidArg(flags,
- _("running and paused flags in %s are mutually "
- "exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING,
+ VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED,
+ error);
if (conn->driver->domainRevertToSnapshot) {
int ret = conn->driver->domainRevertToSnapshot(snapshot, flags);
@@ -1144,14 +1130,9 @@ virDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
virCheckReadOnlyGoto(conn->flags, error);
- if ((flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN) &&
- (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) {
- virReportInvalidArg(flags,
- _("children and children_only flags in %s are "
- "mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN,
+ VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY,
+ error);
if (conn->driver->domainSnapshotDelete) {
int ret = conn->driver->domainSnapshotDelete(snapshot, flags);
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 0bd9274..5482c70 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -911,12 +911,9 @@ virDomainSaveFlags(virDomainPtr domain, const char *to,
virCheckReadOnlyGoto(conn->flags, error);
virCheckNonNullArgGoto(to, error);
- if ((flags & VIR_DOMAIN_SAVE_RUNNING) && (flags &
VIR_DOMAIN_SAVE_PAUSED)) {
- virReportInvalidArg(flags, "%s",
- _("running and paused flags are mutually "
- "exclusive"));
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING,
+ VIR_DOMAIN_SAVE_PAUSED,
+ error);
if (conn->driver->domainSaveFlags) {
int ret;
@@ -1038,12 +1035,9 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const
char *dxml,
virCheckReadOnlyGoto(conn->flags, error);
virCheckNonNullArgGoto(from, error);
- if ((flags & VIR_DOMAIN_SAVE_RUNNING) && (flags &
VIR_DOMAIN_SAVE_PAUSED)) {
- virReportInvalidArg(flags, "%s",
- _("running and paused flags are mutually "
- "exclusive"));
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING,
+ VIR_DOMAIN_SAVE_PAUSED,
+ error);
if (conn->driver->domainRestoreFlags) {
int ret;
@@ -1179,12 +1173,9 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const char *file,
virCheckNonNullArgGoto(file, error);
virCheckNonNullArgGoto(dxml, error);
- if ((flags & VIR_DOMAIN_SAVE_RUNNING) && (flags &
VIR_DOMAIN_SAVE_PAUSED)) {
- virReportInvalidArg(flags, "%s",
- _("running and paused flags are mutually "
- "exclusive"));
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING,
+ VIR_DOMAIN_SAVE_PAUSED,
+ error);
if (conn->driver->domainSaveImageDefineXML) {
int ret;
@@ -1257,23 +1248,9 @@ virDomainCoreDump(virDomainPtr domain, const char *to, unsigned int
flags)
virCheckReadOnlyGoto(conn->flags, error);
virCheckNonNullArgGoto(to, error);
- if ((flags & VIR_DUMP_CRASH) && (flags & VIR_DUMP_LIVE)) {
- virReportInvalidArg(flags, "%s",
- _("crash and live flags are mutually exclusive"));
- goto error;
- }
-
- if ((flags & VIR_DUMP_CRASH) && (flags & VIR_DUMP_RESET)) {
- virReportInvalidArg(flags, "%s",
- _("crash and reset flags are mutually exclusive"));
- goto error;
- }
-
- if ((flags & VIR_DUMP_LIVE) && (flags & VIR_DUMP_RESET)) {
- virReportInvalidArg(flags, "%s",
- _("live and reset flags are mutually exclusive"));
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DUMP_CRASH, VIR_DUMP_LIVE, error);
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DUMP_CRASH, VIR_DUMP_RESET, error);
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DUMP_LIVE, VIR_DUMP_RESET, error);
if (conn->driver->domainCoreDump) {
int ret;
@@ -1355,23 +1332,9 @@ virDomainCoreDumpWithFormat(virDomainPtr domain, const char *to,
goto error;
}
- if ((flags & VIR_DUMP_CRASH) && (flags & VIR_DUMP_LIVE)) {
- virReportInvalidArg(flags, "%s",
- _("crash and live flags are mutually exclusive"));
- goto error;
- }
-
- if ((flags & VIR_DUMP_CRASH) && (flags & VIR_DUMP_RESET)) {
- virReportInvalidArg(flags, "%s",
- _("crash and reset flags are mutually
exclusive"));
- goto error;
- }
-
- if ((flags & VIR_DUMP_LIVE) && (flags & VIR_DUMP_RESET)) {
- virReportInvalidArg(flags, "%s",
- _("live and reset flags are mutually exclusive"));
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DUMP_CRASH, VIR_DUMP_LIVE, error);
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DUMP_CRASH, VIR_DUMP_RESET, error);
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DUMP_LIVE, VIR_DUMP_RESET, error);
if (conn->driver->domainCoreDumpWithFormat) {
int ret;
@@ -2176,15 +2139,10 @@ virDomainGetMemoryParameters(virDomainPtr domain,
VIR_DRV_FEATURE_TYPED_PARAM_STRING))
flags |= VIR_TYPED_PARAM_STRING_OKAY;
- /* At most one of these two flags should be set. */
- if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
- (flags & VIR_DOMAIN_AFFECT_CONFIG)) {
- virReportInvalidArg(flags,
- _("flags 'affect live' and 'affect
config' in %s "
- "are mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_LIVE,
+ VIR_DOMAIN_AFFECT_CONFIG,
+ error);
+
conn = domain->conn;
if (conn->driver->domainGetMemoryParameters) {
@@ -2420,15 +2378,10 @@ virDomainGetBlkioParameters(virDomainPtr domain,
VIR_DRV_FEATURE_TYPED_PARAM_STRING))
flags |= VIR_TYPED_PARAM_STRING_OKAY;
- /* At most one of these two flags should be set. */
- if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
- (flags & VIR_DOMAIN_AFFECT_CONFIG)) {
- virReportInvalidArg(flags,
- _("flags 'affect live' and 'affect
config' in %s "
- "are mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_LIVE,
+ VIR_DOMAIN_AFFECT_CONFIG,
+ error);
+
conn = domain->conn;
if (conn->driver->domainGetBlkioParameters) {
@@ -3578,14 +3531,9 @@ virDomainMigrate(virDomainPtr domain,
virCheckConnectGoto(dconn, error);
virCheckReadOnlyGoto(dconn->flags, error);
- if (flags & VIR_MIGRATE_NON_SHARED_DISK &&
- flags & VIR_MIGRATE_NON_SHARED_INC) {
- virReportInvalidArg(flags,
- _("flags 'shared disk' and 'shared
incremental' "
- "in %s are mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK,
+ VIR_MIGRATE_NON_SHARED_INC,
+ error);
if (flags & VIR_MIGRATE_OFFLINE) {
if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
@@ -3807,14 +3755,9 @@ virDomainMigrate2(virDomainPtr domain,
virCheckConnectGoto(dconn, error);
virCheckReadOnlyGoto(dconn->flags, error);
- if (flags & VIR_MIGRATE_NON_SHARED_DISK &&
- flags & VIR_MIGRATE_NON_SHARED_INC) {
- virReportInvalidArg(flags,
- _("flags 'shared disk' and 'shared
incremental' "
- "in %s are mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK,
+ VIR_MIGRATE_NON_SHARED_INC,
+ error);
if (flags & VIR_MIGRATE_OFFLINE) {
if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
@@ -3987,14 +3930,10 @@ virDomainMigrate3(virDomainPtr domain,
virCheckConnectGoto(dconn, error);
virCheckReadOnlyGoto(dconn->flags, error);
- if (flags & VIR_MIGRATE_NON_SHARED_DISK &&
- flags & VIR_MIGRATE_NON_SHARED_INC) {
- virReportInvalidArg(flags,
- _("flags 'shared disk' and 'shared
incremental' "
- "in %s are mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK,
+ VIR_MIGRATE_NON_SHARED_INC,
+ error);
+
if (flags & VIR_MIGRATE_PEER2PEER) {
virReportInvalidArg(flags, "%s",
_("use virDomainMigrateToURI3 for peer-to-peer "
@@ -4210,14 +4149,9 @@ virDomainMigrateToURI(virDomainPtr domain,
virCheckNonNullArgGoto(duri, error);
- if (flags & VIR_MIGRATE_NON_SHARED_DISK &&
- flags & VIR_MIGRATE_NON_SHARED_INC) {
- virReportInvalidArg(flags,
- _("flags 'shared disk' and 'shared
incremental' "
- "in %s are mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK,
+ VIR_MIGRATE_NON_SHARED_INC,
+ error);
if (flags & VIR_MIGRATE_OFFLINE &&
!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
@@ -4370,14 +4304,9 @@ virDomainMigrateToURI2(virDomainPtr domain,
virCheckDomainReturn(domain, -1);
virCheckReadOnlyGoto(domain->conn->flags, error);
- if (flags & VIR_MIGRATE_NON_SHARED_DISK &&
- flags & VIR_MIGRATE_NON_SHARED_INC) {
- virReportInvalidArg(flags,
- _("flags 'shared disk' and 'shared
incremental' "
- "in %s are mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK,
+ VIR_MIGRATE_NON_SHARED_INC,
+ error);
if (flags & VIR_MIGRATE_PEER2PEER) {
if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
@@ -4479,14 +4408,9 @@ virDomainMigrateToURI3(virDomainPtr domain,
virCheckDomainReturn(domain, -1);
virCheckReadOnlyGoto(domain->conn->flags, error);
- if (flags & VIR_MIGRATE_NON_SHARED_DISK &&
- flags & VIR_MIGRATE_NON_SHARED_INC) {
- virReportInvalidArg(flags,
- _("flags 'shared disk' and 'shared
incremental' "
- "in %s are mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MIGRATE_NON_SHARED_DISK,
+ VIR_MIGRATE_NON_SHARED_INC,
+ error);
compat = virTypedParamsCheck(params, nparams, compatParams,
ARRAY_CARDINALITY(compatParams));
@@ -5526,15 +5450,10 @@ virDomainGetSchedulerParametersFlags(virDomainPtr domain,
VIR_DRV_FEATURE_TYPED_PARAM_STRING))
flags |= VIR_TYPED_PARAM_STRING_OKAY;
- /* At most one of these two flags should be set. */
- if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
- (flags & VIR_DOMAIN_AFFECT_CONFIG)) {
- virReportInvalidArg(flags,
- _("flags 'affect live' and 'affect
config' in %s "
- "are mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_LIVE,
+ VIR_DOMAIN_AFFECT_CONFIG,
+ error);
+
conn = domain->conn;
if (conn->driver->domainGetSchedulerParametersFlags) {
@@ -6295,14 +6214,7 @@ virDomainMemoryPeek(virDomainPtr dom,
* because of incompatible licensing.
*/
- /* Exactly one of these two flags must be set. */
- if (!(flags & VIR_MEMORY_VIRTUAL) == !(flags & VIR_MEMORY_PHYSICAL)) {
- virReportInvalidArg(flags,
- _("flags in %s must include VIR_MEMORY_VIRTUAL or
"
- "VIR_MEMORY_PHYSICAL"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_MEMORY_VIRTUAL, VIR_MEMORY_PHYSICAL, error);
/* Allow size == 0 as an access test. */
if (size > 0)
@@ -7340,14 +7252,21 @@ virDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus,
virCheckDomainReturn(domain, -1);
virCheckReadOnlyGoto(domain->conn->flags, error);
- if (flags & VIR_DOMAIN_VCPU_GUEST &&
- flags & VIR_DOMAIN_VCPU_MAXIMUM) {
- virReportInvalidArg(flags,
- _("flags 'VIR_DOMAIN_VCPU_MAXIMUM' and "
- "'VIR_DOMAIN_VCPU_GUEST' in '%s' are
mutually "
- "exclusive"), __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_CURRENT,
+ VIR_DOMAIN_AFFECT_LIVE,
+ error);
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_CURRENT,
+ VIR_DOMAIN_AFFECT_CONFIG,
+ error);
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_VCPU_GUEST,
+ VIR_DOMAIN_AFFECT_CONFIG,
+ error);
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_VCPU_GUEST,
+ VIR_DOMAIN_VCPU_MAXIMUM,
+ error);
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_CONFIG,
+ VIR_DOMAIN_VCPU_MAXIMUM,
+ error);
virCheckNonZeroArgGoto(nvcpus, error);
@@ -7416,15 +7335,9 @@ virDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags)
if (flags & VIR_DOMAIN_VCPU_GUEST)
virCheckReadOnlyGoto(conn->flags, error);
- /* At most one of these two flags should be set. */
- if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
- (flags & VIR_DOMAIN_AFFECT_CONFIG)) {
- virReportInvalidArg(flags,
- _("flags 'affect live' and 'affect
config' in %s "
- "are mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_LIVE,
+ VIR_DOMAIN_AFFECT_CONFIG,
+ error);
if (conn->driver->domainGetVcpusFlags) {
int ret;
@@ -7622,15 +7535,9 @@ virDomainGetVcpuPinInfo(virDomainPtr domain, int ncpumaps,
goto error;
}
- /* At most one of these two flags should be set. */
- if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
- (flags & VIR_DOMAIN_AFFECT_CONFIG)) {
- virReportInvalidArg(flags,
- _("flags 'affect live' and 'affect
config' in %s "
- "are mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_LIVE,
+ VIR_DOMAIN_AFFECT_CONFIG,
+ error);
if (conn->driver->domainGetVcpuPinInfo) {
int ret;
@@ -7752,15 +7659,10 @@ virDomainGetEmulatorPinInfo(virDomainPtr domain, unsigned char
*cpumap,
virCheckNonNullArgGoto(cpumap, error);
virCheckPositiveArgGoto(maplen, error);
- /* At most one of these two flags should be set. */
- if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
- (flags & VIR_DOMAIN_AFFECT_CONFIG)) {
- virReportInvalidArg(flags,
- _("flags 'affect live' and 'affect
config' in %s "
- "are mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_LIVE,
+ VIR_DOMAIN_AFFECT_CONFIG,
+ error);
+
conn = domain->conn;
if (conn->driver->domainGetEmulatorPinInfo) {
@@ -7922,15 +7824,9 @@ virDomainGetIOThreadsInfo(virDomainPtr dom,
virCheckNonNullArgGoto(info, error);
*info = NULL;
- /* At most one of these two flags should be set. */
- if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
- (flags & VIR_DOMAIN_AFFECT_CONFIG)) {
- virReportInvalidArg(flags,
- _("flags 'affect live' and 'affect
config' in %s "
- "are mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_LIVE,
+ VIR_DOMAIN_AFFECT_CONFIG,
+ error);
if (dom->conn->driver->domainGetIOThreadsInfo) {
int ret;
@@ -8257,14 +8153,9 @@ virDomainGetMetadata(virDomainPtr domain,
virCheckDomainReturn(domain, NULL);
- if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
- (flags & VIR_DOMAIN_AFFECT_CONFIG)) {
- virReportInvalidArg(flags,
- _("flags 'affect live' and 'affect
config' in %s "
- "are mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_LIVE,
+ VIR_DOMAIN_AFFECT_CONFIG,
+ error);
switch (type) {
case VIR_DOMAIN_METADATA_TITLE:
@@ -9309,13 +9200,9 @@ virDomainManagedSave(virDomainPtr dom, unsigned int flags)
virCheckReadOnlyGoto(conn->flags, error);
- if ((flags & VIR_DOMAIN_SAVE_RUNNING) && (flags &
VIR_DOMAIN_SAVE_PAUSED)) {
- virReportInvalidArg(flags,
- _("running and paused flags in %s are mutually "
- "exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING,
+ VIR_DOMAIN_SAVE_PAUSED,
+ error);
if (conn->driver->domainManagedSave) {
int ret;
@@ -10486,15 +10373,10 @@ virDomainGetBlockIoTune(virDomainPtr dom,
VIR_DRV_FEATURE_TYPED_PARAM_STRING))
flags |= VIR_TYPED_PARAM_STRING_OKAY;
- /* At most one of these two flags should be set. */
- if ((flags & VIR_DOMAIN_AFFECT_LIVE) &&
- (flags & VIR_DOMAIN_AFFECT_CONFIG)) {
- virReportInvalidArg(flags,
- _("flags 'affect live' and 'affect
config' in %s "
- "are mutually exclusive"),
- __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_LIVE,
+ VIR_DOMAIN_AFFECT_CONFIG,
+ error);
+
conn = dom->conn;
if (conn->driver->domainGetBlockIoTune) {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6d9217b..cc6656d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2015,13 +2015,8 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
virCheckFlags(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN |
VIR_DOMAIN_REBOOT_GUEST_AGENT, -1);
- /* At most one of these two flags should be set. */
- if ((flags & VIR_DOMAIN_REBOOT_ACPI_POWER_BTN) &&
- (flags & VIR_DOMAIN_REBOOT_GUEST_AGENT)) {
- virReportInvalidArg(flags, "%s",
- _("flags for acpi power button and guest agent are
mutually exclusive"));
- return -1;
- }
+ VIR_EXCLUSIVE_FLAGS_RET(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN,
+ VIR_DOMAIN_REBOOT_GUEST_AGENT);
if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index ba928d8..3689e8e 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -441,13 +441,9 @@ virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED,
virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE |
VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret);
- if (flags == (VIR_STORAGE_POOL_BUILD_OVERWRITE |
- VIR_STORAGE_POOL_BUILD_NO_OVERWRITE)) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("Overwrite and no overwrite flags"
- " are mutually exclusive"));
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_STORAGE_POOL_BUILD_OVERWRITE,
+ VIR_STORAGE_POOL_BUILD_NO_OVERWRITE,
+ error);
if (flags & VIR_STORAGE_POOL_BUILD_OVERWRITE) {
ok_to_mklabel = true;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 35385db..9ce97d6 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -774,14 +774,9 @@ virStorageBackendFileSystemBuild(virConnectPtr conn
ATTRIBUTE_UNUSED,
virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE |
VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret);
- if (flags == (VIR_STORAGE_POOL_BUILD_OVERWRITE |
- VIR_STORAGE_POOL_BUILD_NO_OVERWRITE)) {
-
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("Overwrite and no overwrite flags"
- " are mutually exclusive"));
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_STORAGE_POOL_BUILD_OVERWRITE,
+ VIR_STORAGE_POOL_BUILD_NO_OVERWRITE,
+ error);
if (VIR_STRDUP(parent, pool->def->target.path) < 0)
goto error;
--
2.0.5
10:50 a.m.
New subject: [libvirt] [PATCH 2/6] use new macro helpers to check exclusive flags
On Fri, Mar 20, 2015 at 15:39:00 +0100, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/libvirt-domain-snapshot.c | 45 ++----
src/libvirt-domain.c | 288 +++++++++++--------------------------
src/qemu/qemu_driver.c | 9 +-
src/storage/storage_backend_disk.c | 10 +-
src/storage/storage_backend_fs.c | 11 +-
5 files changed, 106 insertions(+), 257 deletions(-)
I don't object to this change, but as this changes a lot of error
messages I'd like to have a second opinion on this.
Peter
11:22 a.m.
New subject: [libvirt] [PATCH 2/6] use new macro helpers to check exclusive flags
On Fri, Mar 20, 2015 at 15:39:00 +0100, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/libvirt-domain-snapshot.c | 45 ++----
src/libvirt-domain.c | 288 +++++++++++--------------------------
src/qemu/qemu_driver.c | 9 +-
src/storage/storage_backend_disk.c | 10 +-
src/storage/storage_backend_fs.c | 11 +-
5 files changed, 106 insertions(+), 257 deletions(-)
@@ -7340,14 +7252,21 @@ virDomainSetVcpusFlags(virDomainPtr domain,
unsigned int nvcpus,
virCheckDomainReturn(domain, -1);
virCheckReadOnlyGoto(domain->conn->flags, error);
- if (flags & VIR_DOMAIN_VCPU_GUEST &&
- flags & VIR_DOMAIN_VCPU_MAXIMUM) {
- virReportInvalidArg(flags,
- _("flags 'VIR_DOMAIN_VCPU_MAXIMUM' and "
- "'VIR_DOMAIN_VCPU_GUEST' in '%s' are
mutually "
- "exclusive"), __FUNCTION__);
- goto error;
- }
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_CURRENT,
+ VIR_DOMAIN_AFFECT_LIVE,
+ error);
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_CURRENT,
+ VIR_DOMAIN_AFFECT_CONFIG,
+ error);
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_VCPU_GUEST,
+ VIR_DOMAIN_AFFECT_CONFIG,
+ error);
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_VCPU_GUEST,
+ VIR_DOMAIN_VCPU_MAXIMUM,
+ error);
+ VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_CONFIG,
+ VIR_DOMAIN_VCPU_MAXIMUM,
+ error);
By the way, this check here is not enough to check that MAXIMUM is not
actually used with _AFFECT_LIVE.
If you use VIR_DOMAIN_AFFECT_CURRENT and the guest is online, this check
is bypassed as the state of the domain is not known at this point.
This unfortunately needs to be checked after the
virDomainLiveConfigHelperMethod in the actual code.
virCheckNonZeroArgGoto(nvcpus, error);
Peter
9:39 a.m.
New subject: [libvirt] [PATCH 3/6] qemu: cleanup setvcpus
Remove unnecessary maximum variable and also exclusive flags check as
they are now tested by upper layer for all drivers.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_driver.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cc6656d..4942712 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4841,7 +4841,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
virDomainObjPtr vm = NULL;
virDomainDefPtr persistentDef;
int ret = -1;
- bool maximum;
unsigned int maxvcpus = 0;
virQEMUDriverConfigPtr cfg = NULL;
virCapsPtr caps = NULL;
@@ -4897,17 +4896,14 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
goto endjob;
}
- maximum = (flags & VIR_DOMAIN_VCPU_MAXIMUM) != 0;
- flags &= ~VIR_DOMAIN_VCPU_MAXIMUM;
-
if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags,
&persistentDef) < 0)
goto endjob;
/* MAXIMUM cannot be mixed with LIVE. */
- if (maximum && (flags & VIR_DOMAIN_AFFECT_LIVE)) {
+ if ((flags & VIR_DOMAIN_VCPU_MAXIMUM) && (flags &
VIR_DOMAIN_AFFECT_LIVE)) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("cannot adjust maximum on running domain"));
+ _("cannot adjust maximum vcpus on running domain"));
goto endjob;
}
@@ -4917,7 +4913,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
if (!maxvcpus || maxvcpus > persistentDef->maxvcpus)
maxvcpus = persistentDef->maxvcpus;
}
- if (!maximum && nvcpus > maxvcpus) {
+ if (!(flags & VIR_DOMAIN_VCPU_MAXIMUM) && nvcpus > maxvcpus) {
virReportError(VIR_ERR_INVALID_ARG,
_("requested vcpus is greater than max allowable"
" vcpus for the domain: %d > %d"),
@@ -4928,8 +4924,8 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
if (flags & VIR_DOMAIN_VCPU_GUEST) {
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("changing of maximum vCPU count isn't supported
"
- "via guest agent"));
+ _("setting vcpus via guest agent isn't supported
"
+ "on offline domain"));
goto endjob;
}
@@ -4992,7 +4988,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
i);
}
- if (maximum) {
+ if (flags & VIR_DOMAIN_VCPU_MAXIMUM) {
persistentDef->maxvcpus = nvcpus;
if (nvcpus < persistentDef->vcpus)
persistentDef->vcpus = nvcpus;
--
2.0.5
11:01 a.m.
New subject: [libvirt] [PATCH 3/6] qemu: cleanup setvcpus
On Fri, Mar 20, 2015 at 15:39:01 +0100, Pavel Hrdina wrote:
Remove unnecessary maximum variable and also exclusive flags check
as
they are now tested by upper layer for all drivers.
The part about the exclusive flag check is not true.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_driver.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
ACK with commit message fixed.
Peter
12:05 p.m.
New subject: [libvirt] [PATCH 3/6] qemu: cleanup setvcpus
On Tue, Mar 24, 2015 at 05:01:09PM +0100, Peter Krempa wrote:
On Fri, Mar 20, 2015 at 15:39:01 +0100, Pavel Hrdina wrote:
> Remove unnecessary maximum variable and also exclusive flags check as
> they are now tested by upper layer for all drivers.
The part about the exclusive flag check is not true.
>
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
ACK with commit message fixed.
Peter
Thanks, I'll push it shortly.
Pavel
9:39 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: check if domain is really active when do setvcpus with --live
From: Luyao Huang <lhuang(a)redhat.com>
Commit e105dc9 fix setting vcpus for offline domain, but forget check
if vm is active when pass VIR_DOMAIN_AFFECT_LIVE flags.
# virsh setvcpus test3 4 --live
error: Failed to create controller cpu for group: No such file or directory
Add a check if we pass VIR_DOMAIN_AFFECT_LIVE flags.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204006
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_driver.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4942712..c4d96bd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4881,6 +4881,14 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("domain is not running"));
+ goto endjob;
+ }
+ }
+
if (flags & VIR_DOMAIN_AFFECT_LIVE && !(flags &
VIR_DOMAIN_VCPU_GUEST)) {
if (virCgroupNewEmulator(priv->cgroup, false, &cgroup_temp) < 0)
goto endjob;
@@ -4947,12 +4955,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
if (ncpuinfo < 0)
goto endjob;
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("domain is not running"));
- goto endjob;
- }
-
if (qemuAgentUpdateCPUInfo(nvcpus, cpuinfo, ncpuinfo) < 0)
goto endjob;
--
2.0.5
1:27 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: check if domain is really active when do setvcpus with --live
On 03/20/2015 10:39 PM, Pavel Hrdina wrote:
Luyao
From: Luyao Huang <lhuang(a)redhat.com>
Commit e105dc9 fix setting vcpus for offline domain, but forget check
if vm is active when pass VIR_DOMAIN_AFFECT_LIVE flags.
# virsh setvcpus test3 4 --live
error: Failed to create controller cpu for group: No such file or directory
Add a check if we pass VIR_DOMAIN_AFFECT_LIVE flags.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204006
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_driver.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4942712..c4d96bd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4881,6 +4881,14 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("domain is not running"));
+ goto endjob;
+ }
+ }
+
if (flags & VIR_DOMAIN_AFFECT_LIVE && !(flags &
VIR_DOMAIN_VCPU_GUEST)) {
if (virCgroupNewEmulator(priv->cgroup, false, &cgroup_temp) < 0)
goto endjob;
@@ -4947,12 +4955,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
if (ncpuinfo < 0)
goto endjob;
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("domain is not running"));
- goto endjob;
- }
-
if (qemuAgentUpdateCPUInfo(nvcpus, cpuinfo, ncpuinfo) < 0)
goto endjob;
Sorry, i forgot there is a function virDomainLiveConfigHelperMethod(),
move this function just after qemuDomainObjBeginJob() maybe a good way
to fix this issue. Also virDomainLiveConfigHelperMethod() may change
flags, so it should be done more early (as soon as possible after set a
lock to vm).
4:31 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: check if domain is really active when do setvcpus with --live
On Tue, Mar 24, 2015 at 02:27:42PM +0800, lhuang wrote:
On 03/20/2015 10:39 PM, Pavel Hrdina wrote:
> From: Luyao Huang <lhuang(a)redhat.com>
>
> Commit e105dc9 fix setting vcpus for offline domain, but forget check
> if vm is active when pass VIR_DOMAIN_AFFECT_LIVE flags.
>
> # virsh setvcpus test3 4 --live
> error: Failed to create controller cpu for group: No such file or directory
>
> Add a check if we pass VIR_DOMAIN_AFFECT_LIVE flags.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204006
>
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 4942712..c4d96bd 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4881,6 +4881,14 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int
nvcpus,
> if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> goto cleanup;
>
> + if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> + if (!virDomainObjIsActive(vm)) {
> + virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("domain is not running"));
> + goto endjob;
> + }
> + }
> +
> if (flags & VIR_DOMAIN_AFFECT_LIVE && !(flags &
VIR_DOMAIN_VCPU_GUEST)) {
> if (virCgroupNewEmulator(priv->cgroup, false, &cgroup_temp) <
0)
> goto endjob;
> @@ -4947,12 +4955,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int
nvcpus,
> if (ncpuinfo < 0)
> goto endjob;
>
> - if (!virDomainObjIsActive(vm)) {
> - virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> - _("domain is not running"));
> - goto endjob;
> - }
> -
> if (qemuAgentUpdateCPUInfo(nvcpus, cpuinfo, ncpuinfo) < 0)
> goto endjob;
Sorry, i forgot there is a function virDomainLiveConfigHelperMethod(),
move this function just after qemuDomainObjBeginJob() maybe a good way
to fix this issue. Also virDomainLiveConfigHelperMethod() may change
flags, so it should be done more early (as soon as possible after set a
lock to vm).
I've already sent a patch 7/6 to move that function. I realized that right
after I've sent this series to mailing list.
Pavel
>
Luyao
4:39 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: check if domain is really active when do setvcpus with --live
On 03/24/2015 05:31 PM, Pavel Hrdina wrote:
On Tue, Mar 24, 2015 at 02:27:42PM +0800, lhuang wrote:
> On 03/20/2015 10:39 PM, Pavel Hrdina wrote:
>> From: Luyao Huang <lhuang(a)redhat.com>
>>
>> Commit e105dc9 fix setting vcpus for offline domain, but forget check
>> if vm is active when pass VIR_DOMAIN_AFFECT_LIVE flags.
>>
>> # virsh setvcpus test3 4 --live
>> error: Failed to create controller cpu for group: No such file or directory
>>
>> Add a check if we pass VIR_DOMAIN_AFFECT_LIVE flags.
>>
>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204006
>>
>> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
>> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
>> ---
>> src/qemu/qemu_driver.c | 14 ++++++++------
>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 4942712..c4d96bd 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -4881,6 +4881,14 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int
nvcpus,
>> if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
>> goto cleanup;
>>
>> + if (flags & VIR_DOMAIN_AFFECT_LIVE) {
>> + if (!virDomainObjIsActive(vm)) {
>> + virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> + _("domain is not running"));
>> + goto endjob;
>> + }
>> + }
>> +
>> if (flags & VIR_DOMAIN_AFFECT_LIVE && !(flags &
VIR_DOMAIN_VCPU_GUEST)) {
>> if (virCgroupNewEmulator(priv->cgroup, false, &cgroup_temp)
< 0)
>> goto endjob;
>> @@ -4947,12 +4955,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int
nvcpus,
>> if (ncpuinfo < 0)
>> goto endjob;
>>
>> - if (!virDomainObjIsActive(vm)) {
>> - virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> - _("domain is not running"));
>> - goto endjob;
>> - }
>> -
>> if (qemuAgentUpdateCPUInfo(nvcpus, cpuinfo, ncpuinfo) < 0)
>> goto endjob;
> Sorry, i forgot there is a function virDomainLiveConfigHelperMethod(),
> move this function just after qemuDomainObjBeginJob() maybe a good way
> to fix this issue. Also virDomainLiveConfigHelperMethod() may change
> flags, so it should be done more early (as soon as possible after set a
> lock to vm).
I've already sent a patch 7/6 to move that function. I realized that right
after I've sent this series to mailing list.
Pavel
Okay, I think i missed patch 7/6 when i looked these patches :)
Thanks for your reply
>>
> Luyao
Luyao
11:15 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: check if domain is really active when do setvcpus with --live
On Fri, Mar 20, 2015 at 15:39:02 +0100, Pavel Hrdina wrote:
From: Luyao Huang <lhuang(a)redhat.com>
Commit e105dc9 fix setting vcpus for offline domain, but forget check
if vm is active when pass VIR_DOMAIN_AFFECT_LIVE flags.
# virsh setvcpus test3 4 --live
error: Failed to create controller cpu for group: No such file or directory
Add a check if we pass VIR_DOMAIN_AFFECT_LIVE flags.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204006
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_driver.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
NACK, after the virDomainLiveConfigHelperMethod gets moved, this is not
logner necessary as it contains the same code.
Peter
9:39 a.m.
New subject: [libvirt] [PATCH 5/6] tools: fix the wrong check when use virsh setvcpus --maximum
From: Luyao Huang <lhuang(a)redhat.com>
We will ignore --maximum option when only use setvcpus with
this option, like this (this error is another issue):
# virsh setvcpus test3 --maximum 10
error: Failed to create controller cpu for group: No such file or directory
this is because we do not set it in flags before we check if there is
a flags set.
Refactor these code and fix the logic.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204033
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
tools/virsh-domain.c | 30 ++++++------------------------
1 file changed, 6 insertions(+), 24 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1d8225c..9430ad9 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6735,6 +6735,8 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
VSH_EXCLUSIVE_OPTIONS_VAR(guest, config);
+ VSH_EXCLUSIVE_OPTIONS_VAR(guest, maximum);
+ VSH_EXCLUSIVE_OPTIONS_VAR(config, maximum);
if (config)
flags |= VIR_DOMAIN_AFFECT_CONFIG;
@@ -6742,9 +6744,8 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_DOMAIN_AFFECT_LIVE;
if (guest)
flags |= VIR_DOMAIN_VCPU_GUEST;
- /* none of the options were specified */
- if (!current && flags == 0)
- flags = -1;
+ if (maximum)
+ flags |= VIR_DOMAIN_VCPU_MAXIMUM;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -6754,30 +6755,11 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
- if (flags == -1) {
+ /* none of the options were specified */
+ if (!current && flags == 0) {
if (virDomainSetVcpus(dom, count) != 0)
goto cleanup;
} else {
- /* If the --maximum flag was given, we need to ensure only the
- --config flag is in effect as well */
- if (maximum) {
- vshDebug(ctl, VSH_ERR_DEBUG, "--maximum flag was given\n");
-
- flags |= VIR_DOMAIN_VCPU_MAXIMUM;
-
- /* If neither the --config nor --live flags were given, OR
- if just the --live flag was given, we need to error out
- warning the user that the --maximum flag can only be used
- with the --config flag */
- if (live || !config) {
-
- /* Warn the user about the invalid flag combination */
- vshError(ctl, _("--maximum must be used with --config only"));
- goto cleanup;
- }
- }
-
- /* Apply the virtual cpu changes */
if (virDomainSetVcpusFlags(dom, count, flags) < 0)
goto cleanup;
}
--
2.0.5
10:33 p.m.
New subject: [libvirt] [PATCH 5/6] tools: fix the wrong check when use virsh setvcpus --maximum
On 03/20/2015 10:39 PM, Pavel Hrdina wrote:
From: Luyao Huang <lhuang(a)redhat.com>
We will ignore --maximum option when only use setvcpus with
this option, like this (this error is another issue):
# virsh setvcpus test3 --maximum 10
error: Failed to create controller cpu for group: No such file or directory
this is because we do not set it in flags before we check if there is
a flags set.
Refactor these code and fix the logic.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204033
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
tools/virsh-domain.c | 30 ++++++------------------------
1 file changed, 6 insertions(+), 24 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1d8225c..9430ad9 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6735,6 +6735,8 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
VSH_EXCLUSIVE_OPTIONS_VAR(guest, config);
+ VSH_EXCLUSIVE_OPTIONS_VAR(guest, maximum);
+ VSH_EXCLUSIVE_OPTIONS_VAR(config, maximum);
'maximum' should be used with 'config', and 'live' and
'maximum' are
mutually exclusive
if (config)
flags |= VIR_DOMAIN_AFFECT_CONFIG;
@@ -6742,9 +6744,8 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_DOMAIN_AFFECT_LIVE;
if (guest)
flags |= VIR_DOMAIN_VCPU_GUEST;
- /* none of the options were specified */
- if (!current && flags == 0)
- flags = -1;
+ if (maximum)
+ flags |= VIR_DOMAIN_VCPU_MAXIMUM;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -6754,30 +6755,11 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
- if (flags == -1) {
+ /* none of the options were specified */
+ if (!current && flags == 0) {
if (virDomainSetVcpus(dom, count) != 0)
goto cleanup;
} else {
- /* If the --maximum flag was given, we need to ensure only the
- --config flag is in effect as well */
- if (maximum) {
- vshDebug(ctl, VSH_ERR_DEBUG, "--maximum flag was given\n");
-
- flags |= VIR_DOMAIN_VCPU_MAXIMUM;
-
- /* If neither the --config nor --live flags were given, OR
- if just the --live flag was given, we need to error out
- warning the user that the --maximum flag can only be used
- with the --config flag */
- if (live || !config) {
-
- /* Warn the user about the invalid flag combination */
- vshError(ctl, _("--maximum must be used with --config
only"));
- goto cleanup;
- }
- }
-
- /* Apply the virtual cpu changes */
if (virDomainSetVcpusFlags(dom, count, flags) < 0)
goto cleanup;
}
--
Regards,
Yanbing Du
3:41 a.m.
New subject: [libvirt] [PATCH 5/6] tools: fix the wrong check when use virsh setvcpus --maximum
On Mon, Mar 23, 2015 at 11:33:50AM +0800, Yanbing Du wrote:
On 03/20/2015 10:39 PM, Pavel Hrdina wrote:
> From: Luyao Huang <lhuang(a)redhat.com>
>
> We will ignore --maximum option when only use setvcpus with
> this option, like this (this error is another issue):
>
> # virsh setvcpus test3 --maximum 10
> error: Failed to create controller cpu for group: No such file or directory
>
> this is because we do not set it in flags before we check if there is
> a flags set.
>
> Refactor these code and fix the logic.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204033
>
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
> tools/virsh-domain.c | 30 ++++++------------------------
> 1 file changed, 6 insertions(+), 24 deletions(-)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 1d8225c..9430ad9 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -6735,6 +6735,8 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
> VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
> VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
> VSH_EXCLUSIVE_OPTIONS_VAR(guest, config);
> + VSH_EXCLUSIVE_OPTIONS_VAR(guest, maximum);
> + VSH_EXCLUSIVE_OPTIONS_VAR(config, maximum);
'maximum' should be used with 'config', and 'live' and
'maximum' are
mutually exclusive
Yes, you're right, I've definitely meant live instead of config. Good catch.
Pavel
>
> if (config)
> flags |= VIR_DOMAIN_AFFECT_CONFIG;
> @@ -6742,9 +6744,8 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
> flags |= VIR_DOMAIN_AFFECT_LIVE;
> if (guest)
> flags |= VIR_DOMAIN_VCPU_GUEST;
> - /* none of the options were specified */
> - if (!current && flags == 0)
> - flags = -1;
> + if (maximum)
> + flags |= VIR_DOMAIN_VCPU_MAXIMUM;
>
> if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
> return false;
> @@ -6754,30 +6755,11 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
> goto cleanup;
> }
>
> - if (flags == -1) {
> + /* none of the options were specified */
> + if (!current && flags == 0) {
> if (virDomainSetVcpus(dom, count) != 0)
> goto cleanup;
> } else {
> - /* If the --maximum flag was given, we need to ensure only the
> - --config flag is in effect as well */
> - if (maximum) {
> - vshDebug(ctl, VSH_ERR_DEBUG, "--maximum flag was given\n");
> -
> - flags |= VIR_DOMAIN_VCPU_MAXIMUM;
> -
> - /* If neither the --config nor --live flags were given, OR
> - if just the --live flag was given, we need to error out
> - warning the user that the --maximum flag can only be used
> - with the --config flag */
> - if (live || !config) {
> -
> - /* Warn the user about the invalid flag combination */
> - vshError(ctl, _("--maximum must be used with --config
only"));
> - goto cleanup;
> - }
> - }
> -
> - /* Apply the virtual cpu changes */
> if (virDomainSetVcpusFlags(dom, count, flags) < 0)
> goto cleanup;
> }
>
--
Regards,
Yanbing Du
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
11:34 a.m.
New subject: [libvirt] [PATCH 5/6] tools: fix the wrong check when use virsh setvcpus --maximum
On Fri, Mar 20, 2015 at 15:39:03 +0100, Pavel Hrdina wrote:
From: Luyao Huang <lhuang(a)redhat.com>
We will ignore --maximum option when only use setvcpus with
this option, like this (this error is another issue):
# virsh setvcpus test3 --maximum 10
error: Failed to create controller cpu for group: No such file or directory
this is because we do not set it in flags before we check if there is
a flags set.
Refactor these code and fix the logic.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204033
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
tools/virsh-domain.c | 30 ++++++------------------------
1 file changed, 6 insertions(+), 24 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1d8225c..9430ad9 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6735,6 +6735,8 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
VSH_EXCLUSIVE_OPTIONS_VAR(guest, config);
+ VSH_EXCLUSIVE_OPTIONS_VAR(guest, maximum);
+ VSH_EXCLUSIVE_OPTIONS_VAR(config, maximum);
As Yanbing pointed out, you want to make live and maximum exclusive.
Additionally this changes semantics, as currently --maximum was possible
if and only if --config was specified, which would make it exclusive
with --current too. This is also implied in the man page.
We have the following options:
1) Make --maximum imply --config and document that properly
2) Make --maximum mutualy exclusive with --current too
3) Allow --maximum and --current and document that it will fail for
online domains
I'm fine with either of those options
Peter
12:09 p.m.
New subject: [libvirt] [PATCH 5/6] tools: fix the wrong check when use virsh setvcpus --maximum
On Tue, Mar 24, 2015 at 05:34:31PM +0100, Peter Krempa wrote:
On Fri, Mar 20, 2015 at 15:39:03 +0100, Pavel Hrdina wrote:
> From: Luyao Huang <lhuang(a)redhat.com>
>
> We will ignore --maximum option when only use setvcpus with
> this option, like this (this error is another issue):
>
> # virsh setvcpus test3 --maximum 10
> error: Failed to create controller cpu for group: No such file or directory
>
> this is because we do not set it in flags before we check if there is
> a flags set.
>
> Refactor these code and fix the logic.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1204033
>
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
> tools/virsh-domain.c | 30 ++++++------------------------
> 1 file changed, 6 insertions(+), 24 deletions(-)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 1d8225c..9430ad9 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -6735,6 +6735,8 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
> VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
> VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
> VSH_EXCLUSIVE_OPTIONS_VAR(guest, config);
> + VSH_EXCLUSIVE_OPTIONS_VAR(guest, maximum);
> + VSH_EXCLUSIVE_OPTIONS_VAR(config, maximum);
As Yanbing pointed out, you want to make live and maximum exclusive.
Additionally this changes semantics, as currently --maximum was possible
if and only if --config was specified, which would make it exclusive
with --current too. This is also implied in the man page.
We have the following options:
1) Make --maximum imply --config and document that properly
2) Make --maximum mutualy exclusive with --current too
3) Allow --maximum and --current and document that it will fail for
online domains
I'm fine with either of those options
There is also 4th option: specify that maximum requires config instead of make
it mutually exclusive with current. This flag requirements are used by some of
the other libvirt APIs. I'll create another set of macros to tell that some
flags are required and send v2.
Thanks for review.
Pavel
Peter
9:39 a.m.
New subject: [libvirt] [PATCH 6/6] qemu: fix set vcpus on host without NUMA
We don't have to modify cpuset.mems on hosts without NUMA. It also
fixes an error message that you get instead of success if you trying
update vcpus of a guest on a host without NUMA.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_driver.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c4d96bd..eb86d68 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4889,7 +4889,8 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
}
}
- if (flags & VIR_DOMAIN_AFFECT_LIVE && !(flags &
VIR_DOMAIN_VCPU_GUEST)) {
+ if (flags & VIR_DOMAIN_AFFECT_LIVE && !(flags &
VIR_DOMAIN_VCPU_GUEST) &&
+ virNumaIsAvailable()) {
if (virCgroupNewEmulator(priv->cgroup, false, &cgroup_temp) < 0)
goto endjob;
--
2.0.5
11:38 a.m.
New subject: [libvirt] [PATCH 6/6] qemu: fix set vcpus on host without NUMA
On Fri, Mar 20, 2015 at 15:39:04 +0100, Pavel Hrdina wrote:
We don't have to modify cpuset.mems on hosts without NUMA. It
also
fixes an error message that you get instead of success if you trying
update vcpus of a guest on a host without NUMA.
Could you add example of the error you are fixing here?
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_driver.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c4d96bd..eb86d68 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4889,7 +4889,8 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
}
}
- if (flags & VIR_DOMAIN_AFFECT_LIVE && !(flags &
VIR_DOMAIN_VCPU_GUEST)) {
+ if (flags & VIR_DOMAIN_AFFECT_LIVE && !(flags &
VIR_DOMAIN_VCPU_GUEST) &&
+ virNumaIsAvailable()) {
if (virCgroupNewEmulator(priv->cgroup, false, &cgroup_temp) < 0)
goto endjob;
ACK.
Peter
12:29 p.m.
New subject: [libvirt] [PATCH 6/6] qemu: fix set vcpus on host without NUMA
On Tue, Mar 24, 2015 at 05:38:37PM +0100, Peter Krempa wrote:
On Fri, Mar 20, 2015 at 15:39:04 +0100, Pavel Hrdina wrote:
> We don't have to modify cpuset.mems on hosts without NUMA. It also
> fixes an error message that you get instead of success if you trying
> update vcpus of a guest on a host without NUMA.
Could you add example of the error you are fixing here?
Sure
>
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index c4d96bd..eb86d68 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4889,7 +4889,8 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int
nvcpus,
> }
> }
>
> - if (flags & VIR_DOMAIN_AFFECT_LIVE && !(flags &
VIR_DOMAIN_VCPU_GUEST)) {
> + if (flags & VIR_DOMAIN_AFFECT_LIVE && !(flags &
VIR_DOMAIN_VCPU_GUEST) &&
> + virNumaIsAvailable()) {
> if (virCgroupNewEmulator(priv->cgroup, false, &cgroup_temp) < 0)
> goto endjob;
ACK.
Peter
Thanks, will push it shortly.
Pavel
3531
days inactive
3535
days old
25 comments
4 participants
participants (4)
-
lhuang -
Pavel Hrdina -
Peter Krempa -
Yanbing Du