On 01/15/2016 09:05 AM, Nikolay Shirokovskiy wrote:
> Uses virDomainLiveConfigHelperMethod or
> virDomainObjUpdateModificationImpact appropriately.
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
> ---
> src/conf/domain_conf.c | 12 +++---
> src/libxl/libxl_driver.c | 97 ++++--------------------------------------------
> src/lxc/lxc_driver.c | 75 +++----------------------------------
> 3 files changed, 19 insertions(+), 165 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a9706b0..e54c097 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2880,13 +2880,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)) {
Not the same check.
A 'transient' domain is running, but has no on disk config.
So if some command (e.g. virsh $dom setmem 20G --config) is issued, we
want to stop that from happening on a transient domain. However, there
may be other commands executed on a transient domain that we want to
allow to happen, thus we cannot change this into an && check. It needs
to be "if attempting to affect config", then if not persistent, then
error [else allow the change to the config].
The rest is libxl specific and while it seems reasonable, I didn't check
each change... I did note there is at least one change which has command
specific logic dealing with flags adjustments not related to active,
live, persistent, config, current, etc. removed which doesn't seem like
it's right...
John
> + virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("transient domains do not have any "
> + "persistent config"));
> + return -1;
> }
>
> return 0;
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index d4e9c2a7..508bae4 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -1440,7 +1440,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 |
> @@ -1456,38 +1455,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;
> - }
VIR_DOMAIN_MEM_MAXIMUM has nothing to do with CONFIG, LIVE, CURRENT...
> -
> - 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 */
> @@ -3680,25 +3650,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,
> @@ -3788,25 +3741,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,
> @@ -3893,25 +3829,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 24b9622..aea39c1 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -4989,43 +4989,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,
> @@ -5117,41 +5096,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;
> @@ -5231,40 +5189,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;
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list