2:38 a.m.
Original name of series was:
Subject: [PATCH 0/3] make virDomainObjGetPersistentDef return only persistent definition
Changes from version1
=====================
Original motivation of the series was fixing virDomainObjGetPersistentDef so
that it would not return configs other than persistent. However the final patch
that does it in the first version is absent in current version. I think the
fix should be done another way and in different series. Thus this version has
only misc cleanups which are better splitted into patches in this version.
A few words why I leave patch for virDomainObjGetPersistentDef. First I should
not return NULL from the function as this value already has different meaning
- memory allocation error and there is code that checks for this error. This
code calls virDomainObjGetPersistentDef unconditionally but later use the
return value only for persistent domains, thus it could get NULL pointer on
function call and treat it as an allocation error.
I think the proper way would be chaning virDomainObjGetPersistentDef so it
could not fail that is not call virDomainObjSetDefTransient. Looks like this
is already true because drivers that distiguish between running/persistent
config call virDomainObjSetDefTransient ealy in the domain start process (and
even two times) so that later calls for virDomainObjSetDefTransient are
unnecessary.
Nikolay Shirokovskiy (4):
virDomainObjUpdateModificationImpact: reduce nesting
libxlDomainSetMemoryFlags : reuse virDomainLiveConfigHelperMethod
lxc, libxl: reuse virDomainObjUpdateModificationImpact
libxlDomainPinVcpuFlags: remove check duplicates
src/conf/domain_conf.c | 12 +++---
src/libxl/libxl_driver.c | 103 ++++-------------------------------------------
src/lxc/lxc_driver.c | 75 +++-------------------------------
3 files changed, 19 insertions(+), 171 deletions(-)
--
1.8.3.1
2:38 a.m.
New subject: [libvirt] [PATCH v2 1/4] domain config: reduce nesting
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/conf/domain_conf.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0d2957b..d6c33f6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2872,13 +2872,11 @@ virDomainObjUpdateModificationImpact(virDomainObjPtr vm,
return -1;
}
- if (*flags & VIR_DOMAIN_AFFECT_CONFIG) {
- if (!vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("transient domains do not have any "
- "persistent config"));
- return -1;
- }
+ if (!vm->persistent && (*flags & VIR_DOMAIN_AFFECT_CONFIG)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("transient domains do not have any "
+ "persistent config"));
+ return -1;
}
return 0;
--
1.8.3.1
2:46 a.m.
New subject: [libvirt] [PATCH v2 1/4] domain config: reduce nesting
Please, disregard this. It's version of the other 1/4 patch which differs only in
commit message.
On 24.02.2016 11:38, Nikolay Shirokovskiy wrote:
Signed-off-by: Nikolay Shirokovskiy
<nshirokovskiy(a)virtuozzo.com>
---
src/conf/domain_conf.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0d2957b..d6c33f6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2872,13 +2872,11 @@ virDomainObjUpdateModificationImpact(virDomainObjPtr vm,
return -1;
}
- if (*flags & VIR_DOMAIN_AFFECT_CONFIG) {
- if (!vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("transient domains do not have any "
- "persistent config"));
- return -1;
- }
+ if (!vm->persistent && (*flags & VIR_DOMAIN_AFFECT_CONFIG)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("transient domains do not have any "
+ "persistent config"));
+ return -1;
}
return 0;
2:38 a.m.
New subject: [libvirt] [PATCH v2 1/4] virDomainObjUpdateModificationImpact: reduce nesting
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/conf/domain_conf.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0d2957b..d6c33f6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2872,13 +2872,11 @@ virDomainObjUpdateModificationImpact(virDomainObjPtr vm,
return -1;
}
- if (*flags & VIR_DOMAIN_AFFECT_CONFIG) {
- if (!vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("transient domains do not have any "
- "persistent config"));
- return -1;
- }
+ if (!vm->persistent && (*flags & VIR_DOMAIN_AFFECT_CONFIG)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("transient domains do not have any "
+ "persistent config"));
+ return -1;
}
return 0;
--
1.8.3.1
9:46 a.m.
New subject: [libvirt] [PATCH v2 1/4] virDomainObjUpdateModificationImpact: reduce nesting
On 02/24/2016 03:38 AM, Nikolay Shirokovskiy wrote:
Signed-off-by: Nikolay Shirokovskiy
<nshirokovskiy(a)virtuozzo.com>
---
src/conf/domain_conf.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
ACK - I've adjusted the commit message mainly to give some more context
and pushed this patch.
John
2:38 a.m.
New subject: [libvirt] [PATCH v2 2/4] libxlDomainSetMemoryFlags : reuse virDomainLiveConfigHelperMethod
Flag expansion is the same as in virDomainObjUpdateModificationImpact
which virDomainLiveConfigHelperMethod calls internally. The difference
is merely in implementation. Original code don't use mask
and handle case when VIR_DOMAIN_MEM_MAXIMUM is set explicitly.
Other parts are no different.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libxl/libxl_driver.c | 34 ++--------------------------------
1 file changed, 2 insertions(+), 32 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 404016e..06feea5 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1444,7 +1444,6 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virDomainObjPtr vm;
virDomainDefPtr persistentDef = NULL;
- bool isActive;
int ret = -1;
virCheckFlags(VIR_DOMAIN_MEM_LIVE |
@@ -1460,38 +1459,9 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
goto cleanup;
- isActive = virDomainObjIsActive(vm);
-
- if (flags == VIR_DOMAIN_MEM_CURRENT) {
- if (isActive)
- flags = VIR_DOMAIN_MEM_LIVE;
- else
- flags = VIR_DOMAIN_MEM_CONFIG;
- }
- if (flags == VIR_DOMAIN_MEM_MAXIMUM) {
- if (isActive)
- flags = VIR_DOMAIN_MEM_LIVE | VIR_DOMAIN_MEM_MAXIMUM;
- else
- flags = VIR_DOMAIN_MEM_CONFIG | VIR_DOMAIN_MEM_MAXIMUM;
- }
-
- if (!isActive && (flags & VIR_DOMAIN_MEM_LIVE)) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot set memory on an inactive domain"));
+ if (virDomainLiveConfigHelperMethod(cfg->caps, driver->xmlopt, vm, &flags,
+ &persistentDef) < 0)
goto endjob;
- }
-
- if (flags & VIR_DOMAIN_MEM_CONFIG) {
- if (!vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot change persistent config of a transient
domain"));
- goto endjob;
- }
- if (!(persistentDef = virDomainObjGetPersistentDef(cfg->caps,
- driver->xmlopt,
- vm)))
- goto endjob;
- }
if (flags & VIR_DOMAIN_MEM_MAXIMUM) {
/* resize the maximum memory */
--
1.8.3.1
9:46 a.m.
New subject: [libvirt] [PATCH v2 2/4] libxlDomainSetMemoryFlags : reuse virDomainLiveConfigHelperMethod
$subj:
Use the "libxl: ..." prefix please
That way someone filtering on libxl patches may pick up on this. I can
fix this for this one...
On 02/24/2016 03:38 AM, Nikolay Shirokovskiy wrote:
Flag expansion is the same as in
virDomainObjUpdateModificationImpact
which virDomainLiveConfigHelperMethod calls internally. The difference
is merely in implementation. Original code don't use mask
and handle case when VIR_DOMAIN_MEM_MAXIMUM is set explicitly.
Other parts are no different.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libxl/libxl_driver.c | 34 ++--------------------------------
1 file changed, 2 insertions(+), 32 deletions(-)
I think what's being done is OK, but just to be save, I've CC'd Jim
Fehlig who typically reviews libxl patches just to be sure before I push
the change.
John
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 404016e..06feea5 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1444,7 +1444,6 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virDomainObjPtr vm;
virDomainDefPtr persistentDef = NULL;
- bool isActive;
int ret = -1;
virCheckFlags(VIR_DOMAIN_MEM_LIVE |
@@ -1460,38 +1459,9 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
goto cleanup;
- isActive = virDomainObjIsActive(vm);
-
- if (flags == VIR_DOMAIN_MEM_CURRENT) {
- if (isActive)
- flags = VIR_DOMAIN_MEM_LIVE;
- else
- flags = VIR_DOMAIN_MEM_CONFIG;
- }
- if (flags == VIR_DOMAIN_MEM_MAXIMUM) {
- if (isActive)
- flags = VIR_DOMAIN_MEM_LIVE | VIR_DOMAIN_MEM_MAXIMUM;
- else
- flags = VIR_DOMAIN_MEM_CONFIG | VIR_DOMAIN_MEM_MAXIMUM;
- }
-
- if (!isActive && (flags & VIR_DOMAIN_MEM_LIVE)) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot set memory on an inactive domain"));
+ if (virDomainLiveConfigHelperMethod(cfg->caps, driver->xmlopt, vm,
&flags,
+ &persistentDef) < 0)
goto endjob;
- }
-
- if (flags & VIR_DOMAIN_MEM_CONFIG) {
- if (!vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot change persistent config of a transient
domain"));
- goto endjob;
- }
- if (!(persistentDef = virDomainObjGetPersistentDef(cfg->caps,
- driver->xmlopt,
- vm)))
- goto endjob;
- }
if (flags & VIR_DOMAIN_MEM_MAXIMUM) {
/* resize the maximum memory */
3 p.m.
New subject: [libvirt] [PATCH v2 2/4] libxlDomainSetMemoryFlags : reuse virDomainLiveConfigHelperMethod
On 03/01/2016 08:46 AM, John Ferlan wrote:
$subj:
Use the "libxl: ..." prefix please
That way someone filtering on libxl patches may pick up on this. I can
fix this for this one...
On 02/24/2016 03:38 AM, Nikolay Shirokovskiy wrote:
> Flag expansion is the same as in virDomainObjUpdateModificationImpact
> which virDomainLiveConfigHelperMethod calls internally. The difference
> is merely in implementation. Original code don't use mask
> and handle case when VIR_DOMAIN_MEM_MAXIMUM is set explicitly.
>
> Other parts are no different.
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
> ---
> src/libxl/libxl_driver.c | 34 ++--------------------------------
> 1 file changed, 2 insertions(+), 32 deletions(-)
>
I think what's being done is OK, but just to be save, I've CC'd Jim
Fehlig who typically reviews libxl patches just to be sure before I push
the change.
The change looks good to me. In fact, it seems to be an improvement over the
existing logic. ACK.
Regards,
Jim
John
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 404016e..06feea5 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -1444,7 +1444,6 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
> libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
> virDomainObjPtr vm;
> virDomainDefPtr persistentDef = NULL;
> - bool isActive;
> int ret = -1;
>
> virCheckFlags(VIR_DOMAIN_MEM_LIVE |
> @@ -1460,38 +1459,9 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
> if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
> goto cleanup;
>
> - isActive = virDomainObjIsActive(vm);
> -
> - if (flags == VIR_DOMAIN_MEM_CURRENT) {
> - if (isActive)
> - flags = VIR_DOMAIN_MEM_LIVE;
> - else
> - flags = VIR_DOMAIN_MEM_CONFIG;
> - }
> - if (flags == VIR_DOMAIN_MEM_MAXIMUM) {
> - if (isActive)
> - flags = VIR_DOMAIN_MEM_LIVE | VIR_DOMAIN_MEM_MAXIMUM;
> - else
> - flags = VIR_DOMAIN_MEM_CONFIG | VIR_DOMAIN_MEM_MAXIMUM;
> - }
> -
> - if (!isActive && (flags & VIR_DOMAIN_MEM_LIVE)) {
> - virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> - _("cannot set memory on an inactive domain"));
> + if (virDomainLiveConfigHelperMethod(cfg->caps, driver->xmlopt, vm,
&flags,
> + &persistentDef) < 0)
> goto endjob;
> - }
> -
> - if (flags & VIR_DOMAIN_MEM_CONFIG) {
> - if (!vm->persistent) {
> - virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> - _("cannot change persistent config of a transient
domain"));
> - goto endjob;
> - }
> - if (!(persistentDef = virDomainObjGetPersistentDef(cfg->caps,
> - driver->xmlopt,
> - vm)))
> - goto endjob;
> - }
>
> if (flags & VIR_DOMAIN_MEM_MAXIMUM) {
> /* resize the maximum memory */
>
2:38 a.m.
New subject: [libvirt] [PATCH v2 3/4] lxc, libxl: reuse virDomainObjUpdateModificationImpact
First fortunately all versions of *_CURRENT, *_CONFIG and *_LIVE flags
are numerically equal. Second libxl functions that don't use masks
for flag expansion forbid extra flags via virCheckFlags before.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libxl/libxl_driver.c | 63 ++++------------------------------------
src/lxc/lxc_driver.c | 75 ++++--------------------------------------------
2 files changed, 12 insertions(+), 126 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 06feea5..7e65ba2 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3660,25 +3660,8 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
goto cleanup;
- if (virDomainObjIsActive(vm)) {
- if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
- flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
- } else {
- if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
- flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
- /* check consistency between flags and the vm state */
- if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is not running"));
- goto endjob;
- }
- }
-
- if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot modify device on transient
domain"));
- goto endjob;
- }
+ if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+ goto endjob;
if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
if (!(dev = virDomainDeviceDefParse(xml, vm->def,
@@ -3768,25 +3751,8 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
goto cleanup;
- if (virDomainObjIsActive(vm)) {
- if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
- flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
- } else {
- if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
- flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
- /* check consistency between flags and the vm state */
- if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is not running"));
- goto endjob;
- }
- }
-
- if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot modify device on transient
domain"));
- goto endjob;
- }
+ if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+ goto endjob;
if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
if (!(dev = virDomainDeviceDefParse(xml, vm->def,
@@ -3873,25 +3839,8 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
if (virDomainUpdateDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
- if (virDomainObjIsActive(vm)) {
- if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
- flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
- } else {
- if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
- flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
- /* check consistency between flags and the vm state */
- if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is not running"));
- goto cleanup;
- }
- }
-
- if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot modify device on transient
domain"));
- goto cleanup;
- }
+ if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+ goto endjob;
if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
if (!(dev = virDomainDeviceDefParse(xml, vm->def,
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 41639ac..316c60c 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4993,43 +4993,22 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
virDomainDefPtr vmdef = NULL;
virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
int ret = -1;
- unsigned int affect;
virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG, -1);
- affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG);
-
if (!(vm = lxcDomObjFromDomain(dom)))
goto cleanup;
if (virDomainAttachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
- if (virDomainObjIsActive(vm)) {
- if (affect == VIR_DOMAIN_AFFECT_CURRENT)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- } else {
- if (affect == VIR_DOMAIN_AFFECT_CURRENT)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- /* check consistency between flags and the vm state */
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot do live update a device on "
- "inactive domain"));
- goto cleanup;
- }
- }
-
if (!(caps = virLXCDriverGetCapabilities(driver, false)))
goto cleanup;
- if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && !vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot modify device on transient domain"));
- goto cleanup;
- }
+ if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+ goto cleanup;
dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
caps, driver->xmlopt,
@@ -5121,41 +5100,20 @@ static int lxcDomainUpdateDeviceFlags(virDomainPtr dom,
virDomainDefPtr vmdef = NULL;
virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
int ret = -1;
- unsigned int affect;
virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG |
VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1);
- affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG);
-
if (!(vm = lxcDomObjFromDomain(dom)))
goto cleanup;
if (virDomainUpdateDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
- if (virDomainObjIsActive(vm)) {
- if (affect == VIR_DOMAIN_AFFECT_CURRENT)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- } else {
- if (affect == VIR_DOMAIN_AFFECT_CURRENT)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- /* check consistency between flags and the vm state */
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot do live update a device on "
- "inactive domain"));
- goto cleanup;
- }
- }
-
- if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && !vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot modify device on transient domain"));
- goto cleanup;
- }
+ if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+ goto cleanup;
if (!(caps = virLXCDriverGetCapabilities(driver, false)))
goto cleanup;
@@ -5235,40 +5193,19 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom,
virDomainDefPtr vmdef = NULL;
virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
int ret = -1;
- unsigned int affect;
virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG, -1);
- affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG);
-
if (!(vm = lxcDomObjFromDomain(dom)))
goto cleanup;
if (virDomainDetachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
- if (virDomainObjIsActive(vm)) {
- if (affect == VIR_DOMAIN_AFFECT_CURRENT)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- } else {
- if (affect == VIR_DOMAIN_AFFECT_CURRENT)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- /* check consistency between flags and the vm state */
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot do live update a device on "
- "inactive domain"));
- goto cleanup;
- }
- }
-
- if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && !vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot modify device on transient domain"));
- goto cleanup;
- }
+ if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+ goto cleanup;
if (!(caps = virLXCDriverGetCapabilities(driver, false)))
goto cleanup;
--
1.8.3.1
9:46 a.m.
New subject: [libvirt] [PATCH v2 3/4] lxc, libxl: reuse virDomainObjUpdateModificationImpact
On 02/24/2016 03:38 AM, Nikolay Shirokovskiy wrote:
First fortunately all versions of *_CURRENT, *_CONFIG and *_LIVE
flags
are numerically equal. Second libxl functions that don't use masks
for flag expansion forbid extra flags via virCheckFlags before.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libxl/libxl_driver.c | 63 ++++------------------------------------
src/lxc/lxc_driver.c | 75 ++++--------------------------------------------
2 files changed, 12 insertions(+), 126 deletions(-)
This should be two separate patches and use of singular libxl: and lxc:
prefixes on the subject line.
#1 For libxl driver changes (there's a bug there too)
#2 for lxc changes
This one I'll ask you to fix the bug and repost as two separate patches
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 06feea5..7e65ba2 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3660,25 +3660,8 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
goto cleanup;
- if (virDomainObjIsActive(vm)) {
- if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
- flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
- } else {
- if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
- flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
- /* check consistency between flags and the vm state */
- if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is not running"));
- goto endjob;
- }
- }
-
- if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot modify device on transient
domain"));
- goto endjob;
- }
+ if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+ goto endjob;
if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
if (!(dev = virDomainDeviceDefParse(xml, vm->def,
@@ -3768,25 +3751,8 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
goto cleanup;
- if (virDomainObjIsActive(vm)) {
- if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
- flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
- } else {
- if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
- flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
- /* check consistency between flags and the vm state */
- if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is not running"));
- goto endjob;
- }
- }
-
- if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot modify device on transient
domain"));
- goto endjob;
- }
+ if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+ goto endjob;
if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
if (!(dev = virDomainDeviceDefParse(xml, vm->def,
@@ -3873,25 +3839,8 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
if (virDomainUpdateDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
- if (virDomainObjIsActive(vm)) {
- if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
- flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
- } else {
- if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
- flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
- /* check consistency between flags and the vm state */
- if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is not running"));
- goto cleanup;
- }
- }
-
- if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot modify device on transient
domain"));
- goto cleanup;
- }
+ if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+ goto endjob;
This fails to compile since endjob doesn't exist in this context.
John
if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
if (!(dev = virDomainDeviceDefParse(xml, vm->def,
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 41639ac..316c60c 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4993,43 +4993,22 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
virDomainDefPtr vmdef = NULL;
virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
int ret = -1;
- unsigned int affect;
virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG, -1);
- affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG);
-
if (!(vm = lxcDomObjFromDomain(dom)))
goto cleanup;
if (virDomainAttachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
- if (virDomainObjIsActive(vm)) {
- if (affect == VIR_DOMAIN_AFFECT_CURRENT)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- } else {
- if (affect == VIR_DOMAIN_AFFECT_CURRENT)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- /* check consistency between flags and the vm state */
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot do live update a device on "
- "inactive domain"));
- goto cleanup;
- }
- }
-
if (!(caps = virLXCDriverGetCapabilities(driver, false)))
goto cleanup;
- if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && !vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot modify device on transient domain"));
- goto cleanup;
- }
+ if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+ goto cleanup;
dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
caps, driver->xmlopt,
@@ -5121,41 +5100,20 @@ static int lxcDomainUpdateDeviceFlags(virDomainPtr dom,
virDomainDefPtr vmdef = NULL;
virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
int ret = -1;
- unsigned int affect;
virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG |
VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1);
- affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG);
-
if (!(vm = lxcDomObjFromDomain(dom)))
goto cleanup;
if (virDomainUpdateDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
- if (virDomainObjIsActive(vm)) {
- if (affect == VIR_DOMAIN_AFFECT_CURRENT)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- } else {
- if (affect == VIR_DOMAIN_AFFECT_CURRENT)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- /* check consistency between flags and the vm state */
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot do live update a device on "
- "inactive domain"));
- goto cleanup;
- }
- }
-
- if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && !vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot modify device on transient domain"));
- goto cleanup;
- }
+ if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+ goto cleanup;
if (!(caps = virLXCDriverGetCapabilities(driver, false)))
goto cleanup;
@@ -5235,40 +5193,19 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom,
virDomainDefPtr vmdef = NULL;
virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
int ret = -1;
- unsigned int affect;
virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG, -1);
- affect = flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG);
-
if (!(vm = lxcDomObjFromDomain(dom)))
goto cleanup;
if (virDomainDetachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
- if (virDomainObjIsActive(vm)) {
- if (affect == VIR_DOMAIN_AFFECT_CURRENT)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- } else {
- if (affect == VIR_DOMAIN_AFFECT_CURRENT)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- /* check consistency between flags and the vm state */
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot do live update a device on "
- "inactive domain"));
- goto cleanup;
- }
- }
-
- if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && !vm->persistent) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot modify device on transient domain"));
- goto cleanup;
- }
+ if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+ goto cleanup;
if (!(caps = virLXCDriverGetCapabilities(driver, false)))
goto cleanup;
1:18 a.m.
New subject: [libvirt] [PATCH v2 3/4] lxc, libxl: reuse virDomainObjUpdateModificationImpact
On 01.03.2016 18:46, John Ferlan wrote:
On 02/24/2016 03:38 AM, Nikolay Shirokovskiy wrote:
> First fortunately all versions of *_CURRENT, *_CONFIG and *_LIVE flags
> are numerically equal. Second libxl functions that don't use masks
> for flag expansion forbid extra flags via virCheckFlags before.
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
> ---
> src/libxl/libxl_driver.c | 63 ++++------------------------------------
> src/lxc/lxc_driver.c | 75 ++++--------------------------------------------
> 2 files changed, 12 insertions(+), 126 deletions(-)
>
This should be two separate patches and use of singular libxl: and lxc:
prefixes on the subject line.
#1 For libxl driver changes (there's a bug there too)
#2 for lxc changes
This one I'll ask you to fix the bug and repost as two separate patches
OK. I'll resend soon.
2:38 a.m.
New subject: [libvirt] [PATCH v2 4/4] libxlDomainPinVcpuFlags: remove check duplicates
virDomainLiveConfigHelperMethod function checks this
condition already.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libxl/libxl_driver.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7e65ba2..1428239 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2315,12 +2315,6 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu,
if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
goto cleanup;
- if ((flags & VIR_DOMAIN_AFFECT_LIVE) && !virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("domain is inactive"));
- goto endjob;
- }
-
if (virDomainLiveConfigHelperMethod(cfg->caps, driver->xmlopt, vm,
&flags, &targetDef) < 0)
goto endjob;
--
1.8.3.1
9:47 a.m.
New subject: [libvirt] [PATCH v2 4/4] libxlDomainPinVcpuFlags: remove check duplicates
On 02/24/2016 03:38 AM, Nikolay Shirokovskiy wrote:
virDomainLiveConfigHelperMethod function checks this
condition already.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libxl/libxl_driver.c | 6 ------
1 file changed, 6 deletions(-)
ACK - I've adjusted the commit message and pushed this one.
John
2:38 a.m.
New subject: [libvirt] [PATCH v2 4/4] libxl: remove check duplicates
virDomainLiveConfigHelperMethod function checks this
condition already.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libxl/libxl_driver.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7e65ba2..1428239 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2315,12 +2315,6 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu,
if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
goto cleanup;
- if ((flags & VIR_DOMAIN_AFFECT_LIVE) && !virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("domain is inactive"));
- goto endjob;
- }
-
if (virDomainLiveConfigHelperMethod(cfg->caps, driver->xmlopt, vm,
&flags, &targetDef) < 0)
goto endjob;
--
1.8.3.1
2:46 a.m.
New subject: [libvirt] [PATCH v2 4/4] libxl: remove check duplicates
Please, disregard this. It's version of the other 1/4 patch which differs only in
commit message.
On 24.02.2016 11:38, Nikolay Shirokovskiy wrote:
virDomainLiveConfigHelperMethod function checks this
condition already.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/libxl/libxl_driver.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7e65ba2..1428239 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2315,12 +2315,6 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu,
if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
goto cleanup;
- if ((flags & VIR_DOMAIN_AFFECT_LIVE) && !virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("domain is inactive"));
- goto endjob;
- }
-
if (virDomainLiveConfigHelperMethod(cfg->caps, driver->xmlopt, vm,
&flags, &targetDef) < 0)
goto endjob;
3067
days inactive
3075
days old
14 comments
3 participants
participants (3)
-
Jim Fehlig -
John Ferlan -
Nikolay Shirokovskiy