12:39 a.m.
Hi all,
This patchset adds --current option to virsh setmem command.
This is based on the discussion at
http://www.redhat.com/archives/libvir-list/2011-March/msg00564.html
*[PATCH 1/3] setmem: introduce VIR_DOMAIN_MEM_CURRENT flag
*[PATCH 2/3] setmem: add virDomainSetMemroyFlags(,,VIR_DOMAIN_MEM_CURRENT) support
to qemu driver
*[PATCH 3/3] setmem: add --current option to virsh setmem command
Best regards,
Taku Izumi
12:47 a.m.
New subject: [libvirt] [PATCH 1/3] setmem: introduce VIR_DOMAIN_MEM_CURRENT flag
This patch introduces VIR_DOMAIN_MEM_CURRENT flag and
modifies virDomainSetMemoryFlags function to support it.
Signed-off-by: Taku Izumi <izumi.taku(a)jp.fujitsu.com>
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 13 +++++++------
2 files changed, 8 insertions(+), 6 deletions(-)
Index: libvirt/include/libvirt/libvirt.h.in
===================================================================
--- libvirt.orig/include/libvirt/libvirt.h.in
+++ libvirt/include/libvirt/libvirt.h.in
@@ -854,6 +854,7 @@ int virDomainGetMemoryParameters(vir
/* Memory size modification flags. */
typedef enum {
+ VIR_DOMAIN_MEM_CURRENT= 0, /* affect current domain */
VIR_DOMAIN_MEM_LIVE = (1 << 0), /* affect active domain */
VIR_DOMAIN_MEM_CONFIG = (1 << 1), /* affect next boot */
} virDomainMemoryModFlags;
Index: libvirt/src/libvirt.c
===================================================================
--- libvirt.orig/src/libvirt.c
+++ libvirt/src/libvirt.c
@@ -2862,10 +2862,12 @@ error:
* to Domain0 i.e. the domain where the application runs.
* This funcation may requires privileged access to the hypervisor.
*
- * @flags must include VIR_DOMAIN_MEM_LIVE to affect a running
- * domain (which may fail if domain is not active), or
- * VIR_DOMAIN_MEM_CONFIG to affect the next boot via the XML
- * description of the domain. Both flags may be set.
+ * @flags may include VIR_DOMAIN_MEM_LIVE or VIR_DOMAIN_MEM_CONFIG.
+ * Both flags may be set. If VIR_DOMAIN_MEM_LIVE is set, the change affects
+ * a running domain and may fail if domain is not active.
+ * If VIR_DOMAIN_MEM_CONFIG is set, the change affects the next boot via
+ * the XML description on the domain. If neither flag is specified
+ * (=VIR_DOMAIN_MEM_CURRENT), behavior is different depending on hypervisor.
*
* Returns 0 in case of success, -1 in case of failure.
*/
@@ -2891,8 +2893,7 @@ virDomainSetMemoryFlags(virDomainPtr dom
goto error;
}
- if (memory < 4096 ||
- (flags & (VIR_DOMAIN_MEM_LIVE | VIR_DOMAIN_MEM_CONFIG)) == 0) {
+ if (memory < 4096) {
virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
goto error;
}
6:23 p.m.
New subject: [libvirt] [PATCH 1/3] setmem: introduce VIR_DOMAIN_MEM_CURRENT flag
On 03/22/2011 11:47 PM, Taku Izumi wrote:
This patch introduces VIR_DOMAIN_MEM_CURRENT flag and
modifies virDomainSetMemoryFlags function to support it.
/* Memory size modification flags. */
typedef enum {
+ VIR_DOMAIN_MEM_CURRENT= 0, /* affect current domain */
I tweaked the spacing here.
VIR_DOMAIN_MEM_LIVE = (1 << 0), /* affect active domain
*/
VIR_DOMAIN_MEM_CONFIG = (1 << 1), /* affect next boot */
} virDomainMemoryModFlags;
Index: libvirt/src/libvirt.c
===================================================================
--- libvirt.orig/src/libvirt.c
+++ libvirt/src/libvirt.c
@@ -2862,10 +2862,12 @@ error:
* to Domain0 i.e. the domain where the application runs.
* This funcation may requires privileged access to the hypervisor.
Oops, let's fix that typo in "funcation" while we're here.
*
- * @flags must include VIR_DOMAIN_MEM_LIVE to affect a running
- * domain (which may fail if domain is not active), or
- * VIR_DOMAIN_MEM_CONFIG to affect the next boot via the XML
- * description of the domain. Both flags may be set.
+ * @flags may include VIR_DOMAIN_MEM_LIVE or VIR_DOMAIN_MEM_CONFIG.
+ * Both flags may be set. If VIR_DOMAIN_MEM_LIVE is set, the change affects
+ * a running domain and may fail if domain is not active.
+ * If VIR_DOMAIN_MEM_CONFIG is set, the change affects the next boot via
+ * the XML description on the domain. If neither flag is specified
+ * (=VIR_DOMAIN_MEM_CURRENT), behavior is different depending on hypervisor.
I tweaked this wording a bit.
*
* Returns 0 in case of success, -1 in case of failure.
*/
@@ -2891,8 +2893,7 @@ virDomainSetMemoryFlags(virDomainPtr dom
goto error;
}
- if (memory < 4096 ||
- (flags & (VIR_DOMAIN_MEM_LIVE | VIR_DOMAIN_MEM_CONFIG)) == 0) {
+ if (memory < 4096) {
virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
goto error;
}
Here's what I squashed in before pushing:
diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in
index c9129bc..d765412 100644
--- i/include/libvirt/libvirt.h.in
+++ w/include/libvirt/libvirt.h.in
@@ -854,9 +854,9 @@ int virDomainGetMemoryParameters(virDomainPtr
domain,
/* Memory size modification flags. */
typedef enum {
- VIR_DOMAIN_MEM_CURRENT= 0, /* affect current domain */
- VIR_DOMAIN_MEM_LIVE = (1 << 0), /* affect active domain */
- VIR_DOMAIN_MEM_CONFIG = (1 << 1), /* affect next boot */
+ VIR_DOMAIN_MEM_CURRENT = 0, /* affect current domain state */
+ VIR_DOMAIN_MEM_LIVE = (1 << 0), /* affect active domain */
+ VIR_DOMAIN_MEM_CONFIG = (1 << 1), /* affect next boot */
} virDomainMemoryModFlags;
diff --git i/src/libvirt.c w/src/libvirt.c
index ee11643..dde4bd4 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -2822,14 +2822,17 @@ error:
* Dynamically change the target amount of physical memory allocated to a
* domain. If domain is NULL, then this change the amount of memory
reserved
* to Domain0 i.e. the domain where the application runs.
- * This funcation may requires privileged access to the hypervisor.
+ * This function may requires privileged access to the hypervisor.
*
* @flags may include VIR_DOMAIN_MEM_LIVE or VIR_DOMAIN_MEM_CONFIG.
* Both flags may be set. If VIR_DOMAIN_MEM_LIVE is set, the change affects
- * a running domain and may fail if domain is not active.
- * If VIR_DOMAIN_MEM_CONFIG is set, the change affects the next boot via
- * the XML description on the domain. If neither flag is specified
- * (=VIR_DOMAIN_MEM_CURRENT), behavior is different depending on
hypervisor.
+ * a running domain and will fail if domain is not active.
+ * If VIR_DOMAIN_MEM_CONFIG is set, the change affects persistent state,
+ * and will fail for transient domains. If neither flag is specified
+ * (that is, @flags is VIR_DOMAIN_MEM_CURRENT), then an inactive domain
+ * modifies persistent setup, while an active domain is
hypervisor-dependent
+ * on whether just live or both live and persistent state is changed.
+ * Not all hypervisors can support all flag combinations.
*
* Returns 0 in case of success, -1 in case of failure.
*/
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
12:48 a.m.
New subject: [libvirt] [PATCH 2/3] setmem: add virDomainSetMemroyFlags(, , VIR_DOMAIN_MEM_CURRENT) support to qemu driver
This patch adds virDomainSetMemroyFlags(,,VIR_DOMAIN_MEM_CURRENT) support
code to qemu driver.
Signed-off-by: Taku Izumi <izumi.taku(a)jp.fujitsu.com>
---
src/qemu/qemu_driver.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
Index: libvirt/src/qemu/qemu_driver.c
===================================================================
--- libvirt.orig/src/qemu/qemu_driver.c
+++ libvirt/src/qemu/qemu_driver.c
@@ -1575,16 +1575,11 @@ static int qemudDomainSetMemoryFlags(vir
qemuDomainObjPrivatePtr priv;
virDomainObjPtr vm;
virDomainDefPtr persistentDef = NULL;
- int ret = -1, r;
+ int ret = -1, r, isActive;
virCheckFlags(VIR_DOMAIN_MEM_LIVE |
VIR_DOMAIN_MEM_CONFIG, -1);
- if ((flags & (VIR_DOMAIN_MEM_LIVE | VIR_DOMAIN_MEM_CONFIG)) == 0) {
- qemuReportError(VIR_ERR_INVALID_ARG,
- _("invalid flag combination: (0x%x)"), flags);
- }
-
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
qemuDriverUnlock(driver);
@@ -1605,7 +1600,16 @@ static int qemudDomainSetMemoryFlags(vir
if (qemuDomainObjBeginJob(vm) < 0)
goto cleanup;
- if (!virDomainObjIsActive(vm) && (flags & VIR_DOMAIN_MEM_LIVE)) {
+ isActive = virDomainObjIsActive(vm);
+
+ if (flags == VIR_DOMAIN_MEM_CURRENT) {
+ if (isActive)
+ flags = VIR_DOMAIN_MEM_LIVE;
+ else
+ flags = VIR_DOMAIN_MEM_CONFIG;
+ }
+
+ if (!isActive && (flags & VIR_DOMAIN_MEM_LIVE)) {
qemuReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
goto endjob;
6:23 p.m.
New subject: [libvirt] [PATCH 2/3] setmem: add virDomainSetMemroyFlags(, , VIR_DOMAIN_MEM_CURRENT) support to qemu driver
Long subject line. 'git shortlog -20' should give you a feel for how to
shorten this; I went with:
setmem: add VIR_DOMAIN_MEM_CURRENT support to qemu
On 03/22/2011 11:48 PM, Taku Izumi wrote:
This patch adds virDomainSetMemroyFlags(,,VIR_DOMAIN_MEM_CURRENT) support
s/Memroy/Memory/
code to qemu driver.
Signed-off-by: Taku Izumi <izumi.taku(a)jp.fujitsu.com>
---
src/qemu/qemu_driver.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
Index: libvirt/src/qemu/qemu_driver.c
===================================================================
--- libvirt.orig/src/qemu/qemu_driver.c
+++ libvirt/src/qemu/qemu_driver.c
@@ -1575,16 +1575,11 @@ static int qemudDomainSetMemoryFlags(vir
qemuDomainObjPrivatePtr priv;
virDomainObjPtr vm;
virDomainDefPtr persistentDef = NULL;
- int ret = -1, r;
+ int ret = -1, r, isActive;
Hmm, based on the name, I would have guessed isActive is better as a bool.
virCheckFlags(VIR_DOMAIN_MEM_LIVE |
VIR_DOMAIN_MEM_CONFIG, -1);
- if ((flags & (VIR_DOMAIN_MEM_LIVE | VIR_DOMAIN_MEM_CONFIG)) == 0) {
- qemuReportError(VIR_ERR_INVALID_ARG,
- _("invalid flag combination: (0x%x)"), flags);
- }
-
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
qemuDriverUnlock(driver);
@@ -1605,7 +1600,16 @@ static int qemudDomainSetMemoryFlags(vir
if (qemuDomainObjBeginJob(vm) < 0)
goto cleanup;
- if (!virDomainObjIsActive(vm) && (flags & VIR_DOMAIN_MEM_LIVE)) {
+ isActive = virDomainObjIsActive(vm);
Surprisingly, virDomainObjIsActive returns int instead of bool. Well,
no longer :)
+
+ if (flags == VIR_DOMAIN_MEM_CURRENT) {
+ if (isActive)
+ flags = VIR_DOMAIN_MEM_LIVE;
+ else
+ flags = VIR_DOMAIN_MEM_CONFIG;
+ }
+
+ if (!isActive && (flags & VIR_DOMAIN_MEM_LIVE)) {
qemuReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
goto endjob;
I squashed this in, then pushed:
diff --git i/src/conf/domain_conf.h w/src/conf/domain_conf.h
index 10e73cb..95bd11e 100644
--- i/src/conf/domain_conf.h
+++ w/src/conf/domain_conf.h
@@ -1172,7 +1172,7 @@ struct _virDomainObjList {
virHashTable *objs;
};
-static inline int
+static inline bool
virDomainObjIsActive(virDomainObjPtr dom)
{
return dom->def->id != -1;
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index c0e706c..6a0bf24 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -1576,7 +1576,8 @@ static int qemudDomainSetMemoryFlags(virDomainPtr
dom, unsigned long newmem,
qemuDomainObjPrivatePtr priv;
virDomainObjPtr vm;
virDomainDefPtr persistentDef = NULL;
- int ret = -1, r, isActive;
+ int ret = -1, r;
+ bool isActive;
virCheckFlags(VIR_DOMAIN_MEM_LIVE |
VIR_DOMAIN_MEM_CONFIG, -1);
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
12:49 a.m.
New subject: [libvirt] [PATCH 3/3] setmem: add --current option to virsh setmem command
This patch adds the new option (--current) to the "virsh setmem" command.
When --current option is specified, it affects a "current" domain.
The word "current" denotes that if a domain is running, it affects
a running domain only; otherwise it affects a persistent domain.
Signed-off-by: Taku Izumi <izumi.taku(a)jp.fujitsu.com>
---
tools/virsh.c | 20 +++++++++++++++-----
tools/virsh.pod | 7 +++++--
2 files changed, 20 insertions(+), 7 deletions(-)
Index: libvirt/tools/virsh.c
===================================================================
--- libvirt.orig/tools/virsh.c
+++ libvirt/tools/virsh.c
@@ -2920,6 +2920,7 @@ static const vshCmdOptDef opts_setmem[]
{"kilobytes", VSH_OT_INT, VSH_OFLAG_REQ, N_("number of kilobytes of
memory")},
{"config", VSH_OT_BOOL, 0, N_("affect next boot")},
{"live", VSH_OT_BOOL, 0, N_("affect running domain")},
+ {"current", VSH_OT_BOOL, 0, N_("affect current domain")},
{NULL, 0, 0, NULL}
};
@@ -2932,14 +2933,23 @@ cmdSetmem(vshControl *ctl, const vshCmd
int ret = TRUE;
int config = vshCommandOptBool(cmd, "config");
int live = vshCommandOptBool(cmd, "live");
+ int current = vshCommandOptBool(cmd, "current");
int flags = 0;
- /* Need to use flags if config was specified, but prefer older api
- * for live-only behavior otherwise */
- if (config) {
- flags = VIR_DOMAIN_MEM_CONFIG;
+ if (current) {
+ if (live || config) {
+ vshError(ctl, "%s", _("--current must be specified
exclusively"));
+ return FALSE;
+ }
+ flags = VIR_DOMAIN_MEM_CURRENT;
+ } else {
+ if (config)
+ flags |= VIR_DOMAIN_MEM_CONFIG;
if (live)
flags |= VIR_DOMAIN_MEM_LIVE;
+ /* neither option is specified */
+ if (!live && !config)
+ flags = -1;
}
if (!vshConnectionUsability(ctl, ctl->conn))
@@ -2972,7 +2982,7 @@ cmdSetmem(vshControl *ctl, const vshCmd
return FALSE;
}
- if (!flags) {
+ if (flags == -1) {
if (virDomainSetMemory(dom, kilobytes) != 0) {
ret = FALSE;
}
Index: libvirt/tools/virsh.pod
===================================================================
--- libvirt.orig/tools/virsh.pod
+++ libvirt/tools/virsh.pod
@@ -578,12 +578,15 @@ Therefore, -1 is a useful shorthand for
B<Note>: The weight and cap parameters are defined only for the
XEN_CREDIT scheduler and are now I<DEPRECATED>.
-=item B<setmem> I<domain-id> B<kilobytes> optional I<--config>
I<--live>
+=item B<setmem> I<domain-id> B<kilobytes> optional I<--config>
I<--live> I<--current>
Change the memory allocation for a guest domain.
If I<--live> is specified, perform a memory balloon of a running guest.
If I<--config> is specified, affect the next boot of a persistent guest.
-Both flags may be given. If neither flag is given, I<--live> is assumed.
+If I<--current> is specified, affect a current guest.
+Both I<--live> and I<--config> flags may be given, but I<--current> is
+exclusive. If neither flag is specified, behavior is different depending
+on hypervisor.
Some hypervisors require a larger granularity than kilobytes, and requests
that are not an even multiple will be rounded up. For example, vSphere/ESX
6:23 p.m.
New subject: [libvirt] [PATCH 3/3] setmem: add --current option to virsh setmem command
On 03/22/2011 11:49 PM, Taku Izumi wrote:
This patch adds the new option (--current) to the "virsh setmem" command.
When --current option is specified, it affects a "current" domain.
The word "current" denotes that if a domain is running, it affects
a running domain only; otherwise it affects a persistent domain.
Signed-off-by: Taku Izumi <izumi.taku(a)jp.fujitsu.com>
---
tools/virsh.c | 20 +++++++++++++++-----
tools/virsh.pod | 7 +++++--
All right - you remembered to update the docs at the same time!
+ if (current) {
+ if (live || config) {
+ vshError(ctl, "%s", _("--current must be specified
exclusively"));
+ return FALSE;
+ }
+ flags = VIR_DOMAIN_MEM_CURRENT;
+ } else {
+ if (config)
+ flags |= VIR_DOMAIN_MEM_CONFIG;
if (live)
flags |= VIR_DOMAIN_MEM_LIVE;
+ /* neither option is specified */
+ if (!live && !config)
+ flags = -1;
}
if (!vshConnectionUsability(ctl, ctl->conn))
@@ -2972,7 +2982,7 @@ cmdSetmem(vshControl *ctl, const vshCmd
return FALSE;
}
- if (!flags) {
+ if (flags == -1) {
if (virDomainSetMemory(dom, kilobytes) != 0) {
Looks reasonable.
+++ libvirt/tools/virsh.pod
@@ -578,12 +578,15 @@ Therefore, -1 is a useful shorthand for
B<Note>: The weight and cap parameters are defined only for the
XEN_CREDIT scheduler and are now I<DEPRECATED>.
-=item B<setmem> I<domain-id> B<kilobytes> optional I<--config>
I<--live>
+=item B<setmem> I<domain-id> B<kilobytes> optional I<--config>
I<--live> I<--current>
I wrapped this at 80 columns.
Change the memory allocation for a guest domain.
If I<--live> is specified, perform a memory balloon of a running guest.
If I<--config> is specified, affect the next boot of a persistent guest.
-Both flags may be given. If neither flag is given, I<--live> is assumed.
+If I<--current> is specified, affect a current guest.
I tweaked this to 'current guest state (active or persistent)'.
+Both I<--live> and I<--config> flags may be given, but
I<--current> is
+exclusive. If neither flag is specified, behavior is different depending
+on hypervisor.
ACK and applied.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
4977
days inactive
4992
days old
6 comments
2 participants
participants (2)
-
Eric Blake -
Taku Izumi