7:07 p.m.
Now, Xen supports at(de)tach-disk, at(de)tach-interface for inactive domains
with --perisitent option. (Using --persisten against live domain will
cause error.)
Now, virsh attach-xxx/detach-xxx cannot modify inactive domains in qemu and
--persistent option just returns error.
This series adds support for modification of inactive domain's disk, interfaces.
Short description
1 ... clean up for Xen's driver
2 ... add my name to AUTHORS
3 ... add --persistent modification support, just a wrapper function.
4 ... add disk support
5 ... add nics support
Thanks,
-Kame
7:09 p.m.
New subject: [libvirt] [PATCH 1/5] report OOMError in virDomainDiskInsert()
From b92569080a25bf0029d637327a87372bff071fae Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
Date: Thu, 3 Mar 2011 09:20:36 +0900
Subject: [PATCH 1/5] report OOMError in virDomainDiskInsert()
Now, virDomainDiskInsert() returns -1 at memory allocation
failure but it should call virReportOOMError() by itself.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
---
src/conf/domain_conf.c | 4 +++-
src/xen/xm_internal.c | 4 +---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c8350c6..4730683 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4859,8 +4859,10 @@ int virDomainDiskInsert(virDomainDefPtr def,
virDomainDiskDefPtr disk)
{
- if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0)
+ if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) {
+ virReportOOMError();
return -1;
+ }
virDomainDiskInsertPreAlloced(def, disk);
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index 7f73588..7437930 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -1385,10 +1385,8 @@ xenXMDomainAttachDeviceFlags(virDomainPtr domain, const char *xml,
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
{
- if (virDomainDiskInsert(def, dev->data.disk) < 0) {
- virReportOOMError();
+ if (virDomainDiskInsert(def, dev->data.disk) < 0)
goto cleanup;
- }
dev->data.disk = NULL;
}
break;
--
1.7.4.1
4:38 p.m.
New subject: [libvirt] [PATCH 1/5] report OOMError in virDomainDiskInsert()
On 03/02/2011 06:09 PM, KAMEZAWA Hiroyuki wrote:
>From b92569080a25bf0029d637327a87372bff071fae Mon Sep 17 00:00:00
2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
Date: Thu, 3 Mar 2011 09:20:36 +0900
Subject: [PATCH 1/5] report OOMError in virDomainDiskInsert()
Now, virDomainDiskInsert() returns -1 at memory allocation
failure but it should call virReportOOMError() by itself.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
---
src/conf/domain_conf.c | 4 +++-
src/xen/xm_internal.c | 4 +---
2 files changed, 4 insertions(+), 4 deletions(-)
This patch looks accurate, but lacks justification (why can't all
callers continue to call virReportOOMError() on failure)? I'm not sure
whether to apply it without knowing why it is needed, as it just looks
like code motion churn in isolation.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
7:41 p.m.
New subject: [libvirt] [PATCH 1/5] report OOMError in virDomainDiskInsert()
On Tue, 15 Mar 2011 15:38:50 -0600
Eric Blake <eblake(a)redhat.com> wrote:
On 03/02/2011 06:09 PM, KAMEZAWA Hiroyuki wrote:
>>From b92569080a25bf0029d637327a87372bff071fae Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
> Date: Thu, 3 Mar 2011 09:20:36 +0900
> Subject: [PATCH 1/5] report OOMError in virDomainDiskInsert()
>
> Now, virDomainDiskInsert() returns -1 at memory allocation
> failure but it should call virReportOOMError() by itself.
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
> ---
> src/conf/domain_conf.c | 4 +++-
> src/xen/xm_internal.c | 4 +---
> 2 files changed, 4 insertions(+), 4 deletions(-)
This patch looks accurate, but lacks justification (why can't all
callers continue to call virReportOOMError() on failure)? I'm not sure
whether to apply it without knowing why it is needed, as it just looks
like code motion churn in isolation.
just comes from my experiece.
Ok, calls in the caller side.
Thanks,
-Kame
8:13 p.m.
New subject: [libvirt] [PATCH 1/5] report OOMError in virDomainDiskInsert()
On Tue, Mar 15, 2011 at 03:38:50PM -0600, Eric Blake wrote:
On 03/02/2011 06:09 PM, KAMEZAWA Hiroyuki wrote:
>>From b92569080a25bf0029d637327a87372bff071fae Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
> Date: Thu, 3 Mar 2011 09:20:36 +0900
> Subject: [PATCH 1/5] report OOMError in virDomainDiskInsert()
>
> Now, virDomainDiskInsert() returns -1 at memory allocation
> failure but it should call virReportOOMError() by itself.
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
> ---
> src/conf/domain_conf.c | 4 +++-
> src/xen/xm_internal.c | 4 +---
> 2 files changed, 4 insertions(+), 4 deletions(-)
This patch looks accurate, but lacks justification (why can't all
callers continue to call virReportOOMError() on failure)? I'm not sure
Sure they can, but on the premise that they are sure there is an OOM
happens. The semantics of virDomainDiskInsert is not clear here, returns
-1 just means a failure but not an OOM. What if virDomainDiskInsert
fails for other reasons, is it still correct for the caller to call
virReportOOMError()?
whether to apply it without knowing why it is needed, as it just
looks
like code motion churn in isolation.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
Thanks,
Hu Tao
8:29 p.m.
New subject: [libvirt] [PATCH 1/5] report OOMError in virDomainDiskInsert()
On 03/15/2011 07:13 PM, Hu Tao wrote:
On Tue, Mar 15, 2011 at 03:38:50PM -0600, Eric Blake wrote:
> On 03/02/2011 06:09 PM, KAMEZAWA Hiroyuki wrote:
>> >From b92569080a25bf0029d637327a87372bff071fae Mon Sep 17 00:00:00 2001
>> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
>> Date: Thu, 3 Mar 2011 09:20:36 +0900
>> Subject: [PATCH 1/5] report OOMError in virDomainDiskInsert()
>>
>> Now, virDomainDiskInsert() returns -1 at memory allocation
>> failure but it should call virReportOOMError() by itself.
>>
>> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
>> ---
>> src/conf/domain_conf.c | 4 +++-
>> src/xen/xm_internal.c | 4 +---
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> This patch looks accurate, but lacks justification (why can't all
> callers continue to call virReportOOMError() on failure)? I'm not sure
Sure they can, but on the premise that they are sure there is an OOM
happens. The semantics of virDomainDiskInsert is not clear here, returns
-1 just means a failure but not an OOM. What if virDomainDiskInsert
fails for other reasons, is it still correct for the caller to call
virReportOOMError()?
Right now, it only returns -1 for OOM (it's only a 4-line method, so
it's easy to audit that statement for truth), and there's only a single
caller. A valid justification would be that later in this patch series
you plan on adding code to virDomainDiskInsert that returns -1 for other
failures, in which case moving the error reporting into
virDomainDiskInsert is absolutely the right thing to do. Or even just
arguing the refactoring point of view: a patch that adds 4 lines and
subtracts 4 lines is hard to see how it avoids duplicated code, whereas
a refactoring that adds 4 lines and subtracts 8 is easier to spot as a
useful consolidation.
But I'm missing all of that justification in the commit comments - is
there a plan for a later patch to be changing semantics? Is it for
consistency with other functions in the same file that have similar
semantics of reporting OOM on failure rather than delegating to the
caller? Is it a refactoring designed to reduce duplication? In other
words, code motion just for the sake of it is hard to understand (it
doesn't necessarily make it wrong, but certainly delays the review), but
code motion with a stated reason is much easier to ack. Even something
like "future patches will add more callers to virDomainDiskInsert, so
doing the code motion now will reduce code duplication of OOM reporting
later" would help.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
8:58 p.m.
New subject: [libvirt] [PATCH 1/5] report OOMError in virDomainDiskInsert()
On Tue, Mar 15, 2011 at 07:29:24PM -0600, Eric Blake wrote:
On 03/15/2011 07:13 PM, Hu Tao wrote:
> On Tue, Mar 15, 2011 at 03:38:50PM -0600, Eric Blake wrote:
>> On 03/02/2011 06:09 PM, KAMEZAWA Hiroyuki wrote:
>>> >From b92569080a25bf0029d637327a87372bff071fae Mon Sep 17 00:00:00 2001
>>> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
>>> Date: Thu, 3 Mar 2011 09:20:36 +0900
>>> Subject: [PATCH 1/5] report OOMError in virDomainDiskInsert()
>>>
>>> Now, virDomainDiskInsert() returns -1 at memory allocation
>>> failure but it should call virReportOOMError() by itself.
>>>
>>> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
>>> ---
>>> src/conf/domain_conf.c | 4 +++-
>>> src/xen/xm_internal.c | 4 +---
>>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> This patch looks accurate, but lacks justification (why can't all
>> callers continue to call virReportOOMError() on failure)? I'm not sure
>
> Sure they can, but on the premise that they are sure there is an OOM
> happens. The semantics of virDomainDiskInsert is not clear here, returns
> -1 just means a failure but not an OOM. What if virDomainDiskInsert
> fails for other reasons, is it still correct for the caller to call
> virReportOOMError()?
Right now, it only returns -1 for OOM (it's only a 4-line method, so
it's easy to audit that statement for truth), and there's only a single
caller. A valid justification would be that later in this patch series
you plan on adding code to virDomainDiskInsert that returns -1 for other
failures, in which case moving the error reporting into
virDomainDiskInsert is absolutely the right thing to do. Or even just
arguing the refactoring point of view: a patch that adds 4 lines and
subtracts 4 lines is hard to see how it avoids duplicated code, whereas
a refactoring that adds 4 lines and subtracts 8 is easier to spot as a
useful consolidation.
But I'm missing all of that justification in the commit comments - is
there a plan for a later patch to be changing semantics? Is it for
consistency with other functions in the same file that have similar
semantics of reporting OOM on failure rather than delegating to the
caller? Is it a refactoring designed to reduce duplication? In other
words, code motion just for the sake of it is hard to understand (it
doesn't necessarily make it wrong, but certainly delays the review), but
code motion with a stated reason is much easier to ack. Even something
like "future patches will add more callers to virDomainDiskInsert, so
doing the code motion now will reduce code duplication of OOM reporting
later" would help.
There is another call of virDomainDiskInsert() in patch 4 of this series.
--
Thanks,
Hu Tao
7:10 p.m.
New subject: [libvirt] [PATCH 2/5] add my name to AUTHORS
From 85ddb9b00fbd14cf3ead92df659bd568a1d9cd27 Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
Date: Thu, 3 Mar 2011 09:22:45 +0900
Subject: [PATCH 2/5] add my name to AUTHORS
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
---
AUTHORS | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/AUTHORS b/AUTHORS
index fb42662..54a99c6 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -156,6 +156,7 @@ Patches have also been contributed by:
Michal Novotny <minovotn(a)redhat.com>
Christophe Fergeau <teuf(a)gnome.org>
Markus Groß <gross(a)univention.de>
+ KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
[....send patches to get your name here....]
--
1.7.4.1
9:18 p.m.
New subject: [libvirt] [PATCH 2/5] add my name to AUTHORS
At 03/03/2011 09:10 AM, KAMEZAWA Hiroyuki Write:
>From 85ddb9b00fbd14cf3ead92df659bd568a1d9cd27 Mon Sep 17 00:00:00
2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
Date: Thu, 3 Mar 2011 09:22:45 +0900
Subject: [PATCH 2/5] add my name to AUTHORS
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
---
AUTHORS | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/AUTHORS b/AUTHORS
index fb42662..54a99c6 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -156,6 +156,7 @@ Patches have also been contributed by:
Michal Novotny <minovotn(a)redhat.com>
Christophe Fergeau <teuf(a)gnome.org>
Markus Groß <gross(a)univention.de>
+ KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
The file AUTHORS has contained your name. Why do you add it again?
[....send patches to get your name here....]
9:21 p.m.
New subject: [libvirt] [PATCH 2/5] add my name to AUTHORS
On Thu, 03 Mar 2011 11:18:15 +0800
Wen Congyang <wency(a)cn.fujitsu.com> wrote:
At 03/03/2011 09:10 AM, KAMEZAWA Hiroyuki Write:
>>From 85ddb9b00fbd14cf3ead92df659bd568a1d9cd27 Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
> Date: Thu, 3 Mar 2011 09:22:45 +0900
> Subject: [PATCH 2/5] add my name to AUTHORS
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
> ---
> AUTHORS | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/AUTHORS b/AUTHORS
> index fb42662..54a99c6 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -156,6 +156,7 @@ Patches have also been contributed by:
> Michal Novotny <minovotn(a)redhat.com>
> Christophe Fergeau <teuf(a)gnome.org>
> Markus Groß <gross(a)univention.de>
> + KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
The file AUTHORS has contained your name. Why do you add it again?
really ???
....Ah, Kamezawa Hiroyuki is in the list.
I usually use KAMEZAWA and my git's signature is KAMEZAWA..
Hmm.
Sorry, please ignore this.
Thanks,
-Kame
10:52 p.m.
New subject: [libvirt] [PATCH 2/5] add my name to AUTHORS
Maybe this was what I needed.
=
From 310080a8a3a28c9d5f36876971c22eda39600d7a Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
Date: Thu, 3 Mar 2011 13:45:15 +0900
Subject: [PATCH] rename Kamezawa as KAMEZAWA.
Note:
maybe sounds strange but I've used this signature for years.
see http://en.wikipedia.org/wiki/Family_name
---
AUTHORS | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/AUTHORS b/AUTHORS
index fb42662..a6e09f4 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -134,7 +134,7 @@ Patches have also been contributed by:
Lai Jiangshan <laijs(a)cn.fujitsu.com>
Harsh Prateek Bora <harsh(a)linux.vnet.ibm.com>
John Morrissey <jwm(a)horde.net>
- Kamezawa Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
+ KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
Wen Congyang <wency(a)cn.fujitsu.com>
Hu Tao <hutao(a)cn.fujitsu.com>
Laurent Lテゥonard <laurent(a)open-minds.org>
--
1.7.4.1
6:22 p.m.
New subject: [libvirt] [PATCH 2/5] add my name to AUTHORS
On 03/02/2011 09:52 PM, KAMEZAWA Hiroyuki wrote:
Maybe this was what I needed.
=
>From 310080a8a3a28c9d5f36876971c22eda39600d7a Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
Date: Thu, 3 Mar 2011 13:45:15 +0900
Subject: [PATCH] rename Kamezawa as KAMEZAWA.
I've applied this.
Laurent Lテゥonard
Wow - that's quite the mojibake. Are you handling AUTHORS and your
email in UTF-8? For that matter, if you wanted to display your name in
native characters instead of a westernized version, we're okay with that
too.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
6:29 p.m.
New subject: [libvirt] [PATCH 2/5] add my name to AUTHORS
On Thu, 03 Mar 2011 17:22:57 -0700
Eric Blake <eblake(a)redhat.com> wrote:
On 03/02/2011 09:52 PM, KAMEZAWA Hiroyuki wrote:
> Maybe this was what I needed.
> =
>>From 310080a8a3a28c9d5f36876971c22eda39600d7a Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
> Date: Thu, 3 Mar 2011 13:45:15 +0900
> Subject: [PATCH] rename Kamezawa as KAMEZAWA.
I've applied this.
Thank you.
> Laurent Lテゥonard
Wow - that's quite the mojibake. Are you handling AUTHORS and your
email in UTF-8? For that matter, if you wanted to display your name in
native characters instead of a westernized version, we're okay with that
too.
It seems my environment was not utf8, and fixed :)
Name in English is okay.
Thank you.
-Kame
7:11 p.m.
New subject: [libvirt] [PATCH 3/5] libvirt/qemu - support persistent modification of devices.
From f7a997e3cd58cfac81e131afdd20c3691267831d Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
Date: Thu, 3 Mar 2011 09:43:07 +0900
Subject: [PATCH 3/5] libivrt/qemu - support persistent modification of devices.
Now, virsh attach-***/detach-*** has --persistent option and
it can update XML definition of inactive domains.
Now, disk and nic are supported in Xen, but none in qemu.
This patch adds a function for permanent modification for qemu.
Following patches will add disk/network suport.
TODO:
- finally, need to do hotplug + XML modification for active domain.
Changelog v3->v4
- fixed trailing white spaces.
- splitted into 2 part and this patch only contains wrappers.
Changelog v2->v3:
- clean ups.
- handle all VIR_DOMAIN_DEVICE_MODIFY_XXX flags.
---
src/qemu/qemu_driver.c | 143 ++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 131 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c9095bb..3248cdc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4099,16 +4099,131 @@ cleanup:
return ret;
}
-static int qemudDomainAttachDeviceFlags(virDomainPtr dom,
+/*
+ * Attach a device given by XML, the change will be persistent
+ * and domain XML definition file is updated.
+ */
+static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef,
+ virDomainDeviceDefPtr newdev)
+{
+
+ /* At first, check device confliction */
+ switch(newdev->type) {
+ default:
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Sorry, the device is not suppored for now"));
+ return -1;
+ }
+
+ return 0;
+}
+
+static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef,
+ virDomainDeviceDefPtr device)
+{
+ switch(device->type) {
+ default:
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Sorry, the device is not suppored for now"));
+ return -1;
+ }
+ return 0;
+}
+
+static int qemuDomainModifyDevicePersistent(virDomainPtr dom,
const char *xml,
- unsigned int flags) {
- if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
- qemuReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot modify the persistent
configuration of a domain"));
+ unsigned int attach, int flags)
+{
+ struct qemud_driver *driver;
+ virDomainDeviceDefPtr device;
+ virDomainDefPtr vmdef;
+ virDomainObjPtr vm;
+ int ret = -1;
+
+ if (!dom || !dom->conn || !dom->name || !xml) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+ _("internal error : %s"), __FUNCTION__);
+ return -1;
+ }
+ /*
+ * When both of MODIFY_CONFIG and MODIFY_LIVE is specified at the same time,
+ * return error for now. We should support this later.
+ */
+ if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Now, cannot modify alive domain and its definition
"
+ "at the same time."));
return -1;
}
- return qemudDomainAttachDevice(dom, xml);
+ driver = dom->conn->privateData;
+ qemuDriverLock(driver);
+ vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+ if (!vm) {
+ qemuReportError(VIR_ERR_NO_DOMAIN, _("cannot find domain
'%s'"),
+ dom->name);
+ goto unlock_out;
+ }
+
+ if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
+ goto unlock_out;
+
+ if (virDomainObjIsActive(vm)) {
+ /*
+ * For now, just allow updating inactive domains. Further development
+ * will allow updating both active domain and its config file at
+ * the same time.
+ */
+ qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("Now, it's unsupported to update active domain's
definition."));
+ goto endjob;
+ }
+
+ vmdef = virDomainObjGetPersistentDef(driver->caps, vm);
+
+ if (!vmdef)
+ goto endjob;
+
+ device = virDomainDeviceDefParse(driver->caps,
+ vmdef, xml, VIR_DOMAIN_XML_INACTIVE);
+ if (!device)
+ goto endjob;
+
+ if (attach)
+ ret = qemuDomainAttachDevicePersistent(vmdef, device);
+ else
+ ret = qemuDomainDetachDevicePersistent(vmdef, device);
+
+ if (!ret) /* save the result */
+ ret = virDomainSaveConfig(driver->configDir, vmdef);
+
+ virDomainDeviceDefFree(device);
+
+endjob:
+ if (qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
+ if (vm)
+ virDomainObjUnlock(vm);
+unlock_out:
+ qemuDriverUnlock(driver);
+ return ret;
+}
+
+static int qemudDomainAttachDeviceFlags(virDomainPtr dom,
+ const char *xml,
+ unsigned int flags)
+{
+ if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG)
+ return qemuDomainModifyDevicePersistent(dom, xml, 1, flags);
+
+ if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE)
+ return qemudDomainAttachDevice(dom, xml);
+
+ qemuReportError(VIR_ERR_INVALID_ARG,
+ _("bad flag: %x only MODIFY_LIVE, MODIFY_CONFIG are supported now"),
+ flags);
+
+ return -1;
}
@@ -4322,13 +4437,17 @@ cleanup:
static int qemudDomainDetachDeviceFlags(virDomainPtr dom,
const char *xml,
unsigned int flags) {
- if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
- qemuReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot modify the persistent
configuration of a domain"));
- return -1;
- }
- return qemudDomainDetachDevice(dom, xml);
+ if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG)
+ return qemuDomainModifyDevicePersistent(dom, xml, 0, flags);
+
+ if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE)
+ return qemudDomainDetachDevice(dom, xml);
+
+ qemuReportError(VIR_ERR_INVALID_ARG,
+ _("bad flag: %x only MODIFY_LIVE, MODIFY_CONFIG are supported now"),
+ flags);
+ return -1;
}
static int qemudDomainGetAutostart(virDomainPtr dom,
--
1.7.4.1
12:29 a.m.
New subject: [libvirt] [PATCH 3/5] libvirt/qemu - support persistent modification of devices.
At 03/03/2011 09:11 AM, KAMEZAWA Hiroyuki Write:
>From f7a997e3cd58cfac81e131afdd20c3691267831d Mon Sep 17 00:00:00
2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
Date: Thu, 3 Mar 2011 09:43:07 +0900
Subject: [PATCH 3/5] libivrt/qemu - support persistent modification of devices.
Now, virsh attach-***/detach-*** has --persistent option and
it can update XML definition of inactive domains.
Now, disk and nic are supported in Xen, but none in qemu.
This patch adds a function for permanent modification for qemu.
Following patches will add disk/network suport.
TODO:
- finally, need to do hotplug + XML modification for active domain.
Changelog v3->v4
- fixed trailing white spaces.
- splitted into 2 part and this patch only contains wrappers.
Changelog v2->v3:
- clean ups.
- handle all VIR_DOMAIN_DEVICE_MODIFY_XXX flags.
---
src/qemu/qemu_driver.c | 143 ++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 131 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c9095bb..3248cdc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4099,16 +4099,131 @@ cleanup:
return ret;
}
-static int qemudDomainAttachDeviceFlags(virDomainPtr dom,
+/*
+ * Attach a device given by XML, the change will be persistent
+ * and domain XML definition file is updated.
+ */
+static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef,
+ virDomainDeviceDefPtr newdev)
+{
+
+ /* At first, check device confliction */
+ switch(newdev->type) {
+ default:
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
When the device is not supported, we should use VIR_ERR_CONFIG_UNSUPPORTED
instead of VIR_ERR_INVALID_ARG.
+ _("Sorry, the device is not suppored
for now"));
+ return -1;
+ }
+
+ return 0;
+}
+
+static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef,
+ virDomainDeviceDefPtr device)
The coding style is wrong: virDomainDeviceDefPtr shoulde be left justified
with virDomainDefPtr.
+{
+ switch(device->type) {
+ default:
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Sorry, the device is not suppored for now"));
+ return -1;
+ }
+ return 0;
+}
+
+static int qemuDomainModifyDevicePersistent(virDomainPtr dom,
const char *xml,
- unsigned int flags) {
- if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
- qemuReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot modify the persistent
configuration of a domain"));
+ unsigned int attach, int flags)
Why do you modify the type of flag?
+{
+ struct qemud_driver *driver;
+ virDomainDeviceDefPtr device;
+ virDomainDefPtr vmdef;
+ virDomainObjPtr vm;
+ int ret = -1;
+
+ if (!dom || !dom->conn || !dom->name || !xml) {
dom and dom->conn does not need check, they are always not null.
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+ _("internal error : %s"), __FUNCTION__);
Do not use __FUNCTION__ here, the macro qemuReportError will do it.
+ return -1;
+ }
+ /*
+ * When both of MODIFY_CONFIG and MODIFY_LIVE is specified at the same time,
+ * return error for now. We should support this later.
+ */
+ if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
Please use VIR_ERR_OPERATION_INVALID instead of VIR_ERR_INVALID_ARG.
+ _("Now, cannot modify alive domain and
its definition "
+ "at the same time."));
return -1;
}
- return qemudDomainAttachDevice(dom, xml);
+ driver = dom->conn->privateData;
+ qemuDriverLock(driver);
+ vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+ if (!vm) {
+ qemuReportError(VIR_ERR_NO_DOMAIN, _("cannot find domain
'%s'"),
+ dom->name);
You use UUID to search the domain, so the error message should be:
no domain with matching uuid '%s'
+ goto unlock_out;
+ }
+
+ if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
+ goto unlock_out;
+
+ if (virDomainObjIsActive(vm)) {
+ /*
+ * For now, just allow updating inactive domains. Further development
+ * will allow updating both active domain and its config file at
+ * the same time.
+ */
+ qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("Now, it's unsupported to update active domain's
definition."));
+ goto endjob;
+ }
+
+ vmdef = virDomainObjGetPersistentDef(driver->caps, vm);
+
+ if (!vmdef)
+ goto endjob;
+
+ device = virDomainDeviceDefParse(driver->caps,
+ vmdef, xml, VIR_DOMAIN_XML_INACTIVE);
+ if (!device)
+ goto endjob;
+
+ if (attach)
+ ret = qemuDomainAttachDevicePersistent(vmdef, device);
+ else
+ ret = qemuDomainDetachDevicePersistent(vmdef, device);
+
+ if (!ret) /* save the result */
+ ret = virDomainSaveConfig(driver->configDir, vmdef);
+
+ virDomainDeviceDefFree(device);
+
+endjob:
+ if (qemuDomainObjEndJob(vm) == 0)
+ vm = NULL;
+ if (vm)
+ virDomainObjUnlock(vm);
When qemuDomainObjBeginJobWithDriver() failed, we should unlock the domain.
So we should do it when we goto unlock_out.
+unlock_out:
+ qemuDriverUnlock(driver);
+ return ret;
+}
+
+static int qemudDomainAttachDeviceFlags(virDomainPtr dom,
+ const char *xml,
+ unsigned int flags)
+{
+ if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG)
+ return qemuDomainModifyDevicePersistent(dom, xml, 1, flags);
+
+ if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE)
+ return qemudDomainAttachDevice(dom, xml);
+
+ qemuReportError(VIR_ERR_INVALID_ARG,
+ _("bad flag: %x only MODIFY_LIVE, MODIFY_CONFIG are supported now"),
+ flags);
+
+ return -1;
}
@@ -4322,13 +4437,17 @@ cleanup:
static int qemudDomainDetachDeviceFlags(virDomainPtr dom,
const char *xml,
unsigned int flags) {
- if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
- qemuReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot modify the persistent
configuration of a domain"));
- return -1;
- }
- return qemudDomainDetachDevice(dom, xml);
+ if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG)
+ return qemuDomainModifyDevicePersistent(dom, xml, 0, flags);
+
+ if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE)
+ return qemudDomainDetachDevice(dom, xml);
+
+ qemuReportError(VIR_ERR_INVALID_ARG,
+ _("bad flag: %x only MODIFY_LIVE, MODIFY_CONFIG are supported now"),
+ flags);
+ return -1;
}
static int qemudDomainGetAutostart(virDomainPtr dom,
8:58 p.m.
New subject: [libvirt] [PATCH 3/5] libvirt/qemu - support persistent modification of devices.
On Mon, 14 Mar 2011 13:29:13 +0800
Wen Congyang <wency(a)cn.fujitsu.com> wrote:
At 03/03/2011 09:11 AM, KAMEZAWA Hiroyuki Write:
>>From f7a997e3cd58cfac81e131afdd20c3691267831d Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
> Date: Thu, 3 Mar 2011 09:43:07 +0900
> Subject: [PATCH 3/5] libivrt/qemu - support persistent modification of devices.
>
> Now, virsh attach-***/detach-*** has --persistent option and
> it can update XML definition of inactive domains.
> Now, disk and nic are supported in Xen, but none in qemu.
>
> This patch adds a function for permanent modification for qemu.
> Following patches will add disk/network suport.
>
> TODO:
> - finally, need to do hotplug + XML modification for active domain.
>
> Changelog v3->v4
> - fixed trailing white spaces.
> - splitted into 2 part and this patch only contains wrappers.
> Changelog v2->v3:
> - clean ups.
> - handle all VIR_DOMAIN_DEVICE_MODIFY_XXX flags.
> ---
> src/qemu/qemu_driver.c | 143 ++++++++++++++++++++++++++++++++++++++++++++----
> 1 files changed, 131 insertions(+), 12 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index c9095bb..3248cdc 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4099,16 +4099,131 @@ cleanup:
> return ret;
> }
>
> -static int qemudDomainAttachDeviceFlags(virDomainPtr dom,
> +/*
> + * Attach a device given by XML, the change will be persistent
> + * and domain XML definition file is updated.
> + */
> +static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef,
> + virDomainDeviceDefPtr newdev)
> +{
> +
> + /* At first, check device confliction */
> + switch(newdev->type) {
> + default:
> + qemuReportError(VIR_ERR_INVALID_ARG, "%s",
When the device is not supported, we should use VIR_ERR_CONFIG_UNSUPPORTED
instead of VIR_ERR_INVALID_ARG.
will change.
> + _("Sorry, the device is not
suppored for now"));
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef,
> + virDomainDeviceDefPtr device)
The coding style is wrong: virDomainDeviceDefPtr shoulde be left justified
with virDomainDefPtr.
Hmm, okay.
> +{
> + switch(device->type) {
> + default:
> + qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("Sorry, the device is not suppored for now"));
> + return -1;
> + }
> + return 0;
> +}
> +
> +static int qemuDomainModifyDevicePersistent(virDomainPtr dom,
> const char *xml,
> - unsigned int flags) {
> - if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
> - qemuReportError(VIR_ERR_OPERATION_INVALID,
> - "%s", _("cannot modify the persistent
configuration of a domain"));
> + unsigned int attach, int flags)
Why do you modify the type of flag?
my bug.
> +{
> + struct qemud_driver *driver;
> + virDomainDeviceDefPtr device;
> + virDomainDefPtr vmdef;
> + virDomainObjPtr vm;
> + int ret = -1;
> +
> + if (!dom || !dom->conn || !dom->name || !xml) {
dom and dom->conn does not need check, they are always not null.
Ok, so, Xen code should be fixed. (This is just a copy-n-paste.)
> + qemuReportError(VIR_ERR_INTERNAL_ERROR,
> + _("internal error : %s"), __FUNCTION__);
Do not use __FUNCTION__ here, the macro qemuReportError will do it.
> + return -1;
> + }
> + /*
> + * When both of MODIFY_CONFIG and MODIFY_LIVE is specified at the same time,
> + * return error for now. We should support this later.
> + */
> + if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
> + qemuReportError(VIR_ERR_INVALID_ARG, "%s",
Please use VIR_ERR_OPERATION_INVALID instead of VIR_ERR_INVALID_ARG.
ok.
> + _("Now, cannot modify alive domain
and its definition "
> + "at the same time."));
> return -1;
> }
>
> - return qemudDomainAttachDevice(dom, xml);
> + driver = dom->conn->privateData;
> + qemuDriverLock(driver);
> + vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> + if (!vm) {
> + qemuReportError(VIR_ERR_NO_DOMAIN, _("cannot find domain
'%s'"),
> + dom->name);
You use UUID to search the domain, so the error message should be:
no domain with matching uuid '%s'
ok.
> + goto unlock_out;
> + }
> +
> + if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
> + goto unlock_out;
> +
> + if (virDomainObjIsActive(vm)) {
> + /*
> + * For now, just allow updating inactive domains. Further development
> + * will allow updating both active domain and its config file at
> + * the same time.
> + */
> + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("Now, it's unsupported to update active domain's
definition."));
> + goto endjob;
> + }
> +
> + vmdef = virDomainObjGetPersistentDef(driver->caps, vm);
> +
> + if (!vmdef)
> + goto endjob;
> +
> + device = virDomainDeviceDefParse(driver->caps,
> + vmdef, xml, VIR_DOMAIN_XML_INACTIVE);
> + if (!device)
> + goto endjob;
> +
> + if (attach)
> + ret = qemuDomainAttachDevicePersistent(vmdef, device);
> + else
> + ret = qemuDomainDetachDevicePersistent(vmdef, device);
> +
> + if (!ret) /* save the result */
> + ret = virDomainSaveConfig(driver->configDir, vmdef);
> +
> + virDomainDeviceDefFree(device);
> +
> +endjob:
> + if (qemuDomainObjEndJob(vm) == 0)
> + vm = NULL;
> + if (vm)
> + virDomainObjUnlock(vm);
When qemuDomainObjBeginJobWithDriver() failed, we should unlock the domain.
So we should do it when we goto unlock_out.
ok.
> +unlock_out:
> + qemuDriverUnlock(driver);
> + return ret;
> +}
> +
> +static int qemudDomainAttachDeviceFlags(virDomainPtr dom,
> + const char *xml,
> + unsigned int flags)
> +{
> + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG)
> + return qemuDomainModifyDevicePersistent(dom, xml, 1, flags);
> +
> + if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE)
> + return qemudDomainAttachDevice(dom, xml);
> +
> + qemuReportError(VIR_ERR_INVALID_ARG,
> + _("bad flag: %x only MODIFY_LIVE, MODIFY_CONFIG are supported
now"),
> + flags);
> +
> + return -1;
> }
>
>
> @@ -4322,13 +4437,17 @@ cleanup:
> static int qemudDomainDetachDeviceFlags(virDomainPtr dom,
> const char *xml,
> unsigned int flags) {
> - if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
> - qemuReportError(VIR_ERR_OPERATION_INVALID,
> - "%s", _("cannot modify the persistent
configuration of a domain"));
> - return -1;
> - }
>
> - return qemudDomainDetachDevice(dom, xml);
> + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG)
> + return qemuDomainModifyDevicePersistent(dom, xml, 0, flags);
> +
> + if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE)
> + return qemudDomainDetachDevice(dom, xml);
> +
> + qemuReportError(VIR_ERR_INVALID_ARG,
> + _("bad flag: %x only MODIFY_LIVE, MODIFY_CONFIG are supported
now"),
> + flags);
> + return -1;
> }
>
> static int qemudDomainGetAutostart(virDomainPtr dom,
Thank you.
-Kame
4:55 p.m.
New subject: [libvirt] [PATCH 3/5] libvirt/qemu - support persistent modification of devices.
On 03/02/2011 06:11 PM, KAMEZAWA Hiroyuki wrote:
>From f7a997e3cd58cfac81e131afdd20c3691267831d Mon Sep 17 00:00:00
2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
Date: Thu, 3 Mar 2011 09:43:07 +0900
Subject: [PATCH 3/5] libivrt/qemu - support persistent modification of devices.
Now, virsh attach-***/detach-*** has --persistent option and
it can update XML definition of inactive domains.
Hmm, looking at the recent additions of other virsh persistent support,
the single --persistent flag of virsh attach-{device,disk,interface} are
a bit limited; --persistent means the same thing as --config, but we may
need to add a counterpart --live option that specifies live only. But
cleaning up virsh to better expose the underlying flags is a separate
patch; this patch only deals with implementing the flags internally.
Now, disk and nic are supported in Xen, but none in qemu.
It would be nice to get all devices supported, but incremental is better
than nothing at all.
This patch adds a function for permanent modification for qemu.
Following patches will add disk/network suport.
TODO:
- finally, need to do hotplug + XML modification for active domain.
Changelog v3->v4
- fixed trailing white spaces.
- splitted into 2 part and this patch only contains wrappers.
Changelog v2->v3:
- clean ups.
- handle all VIR_DOMAIN_DEVICE_MODIFY_XXX flags.
---
src/qemu/qemu_driver.c | 143 ++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 131 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c9095bb..3248cdc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4099,16 +4099,131 @@ cleanup:
return ret;
}
-static int qemudDomainAttachDeviceFlags(virDomainPtr dom,
+/*
+ * Attach a device given by XML, the change will be persistent
+ * and domain XML definition file is updated.
+ */
+static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef,
+ virDomainDeviceDefPtr newdev)
+{
+
+ /* At first, check device confliction */
s/device confliction/for device support/
+ switch(newdev->type) {
+ default:
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Sorry, the device is not suppored for now"));
s/suppored/supported/ (twice)
+
+static int qemuDomainModifyDevicePersistent(virDomainPtr dom,
const char *xml,
Indentation is not consistent.
- unsigned int flags) {
- if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
- qemuReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot modify the persistent
configuration of a domain"));
+ unsigned int attach, int flags)
+{
+ struct qemud_driver *driver;
+ virDomainDeviceDefPtr device;
+ virDomainDefPtr vmdef;
+ virDomainObjPtr vm;
+ int ret = -1;
+
+ if (!dom || !dom->conn || !dom->name || !xml) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+ _("internal error : %s"), __FUNCTION__);
+ return -1;
+ }
+ /*
+ * When both of MODIFY_CONFIG and MODIFY_LIVE is specified at the same time,
+ * return error for now. We should support this later.
+ */
+ if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Now, cannot modify alive domain and its definition
"
+ "at the same time."));
It shouldn't be too hard to attempt supporting this now - after all, we
already did just that with Taku Izumi's patch for virsh setmem.
qemudDomainSetMemoryFlags can be a good reference point for how to
implement a command that first validates all conditions required by the
flags, then modifies the live configuration if requested and still live,
then modifies the persistent configuration if requested.
I think there have been enough findings already (Wen's feedback has also
been useful), as well as quite a bit of upstream churn that will require
rebasing aspects of your patch, that I'll wait for v5 of this series; if
you use 'git send-email --subject-prefix=PATCHv5', that will make it
easier to recognize from the subject line that it is a resend.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
7:44 p.m.
New subject: [libvirt] [PATCH 3/5] libvirt/qemu - support persistent modification of devices.
On Tue, 15 Mar 2011 15:55:51 -0600
Eric Blake <eblake(a)redhat.com> wrote:
On 03/02/2011 06:11 PM, KAMEZAWA Hiroyuki wrote:
>>From f7a997e3cd58cfac81e131afdd20c3691267831d Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
> Date: Thu, 3 Mar 2011 09:43:07 +0900
> Subject: [PATCH 3/5] libivrt/qemu - support persistent modification of devices.
>
> Now, virsh attach-***/detach-*** has --persistent option and
> it can update XML definition of inactive domains.
Hmm, looking at the recent additions of other virsh persistent support,
the single --persistent flag of virsh attach-{device,disk,interface} are
a bit limited; --persistent means the same thing as --config, but we may
need to add a counterpart --live option that specifies live only. But
cleaning up virsh to better expose the underlying flags is a separate
patch; this patch only deals with implementing the flags internally.
yes.
> Now, disk and nic are supported in Xen, but none in qemu.
It would be nice to get all devices supported, but incremental is better
than nothing at all.
>
> This patch adds a function for permanent modification for qemu.
> Following patches will add disk/network suport.
>
> TODO:
> - finally, need to do hotplug + XML modification for active domain.
>
> Changelog v3->v4
> - fixed trailing white spaces.
> - splitted into 2 part and this patch only contains wrappers.
> Changelog v2->v3:
> - clean ups.
> - handle all VIR_DOMAIN_DEVICE_MODIFY_XXX flags.
> ---
> src/qemu/qemu_driver.c | 143 ++++++++++++++++++++++++++++++++++++++++++++----
> 1 files changed, 131 insertions(+), 12 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index c9095bb..3248cdc 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4099,16 +4099,131 @@ cleanup:
> return ret;
> }
>
> -static int qemudDomainAttachDeviceFlags(virDomainPtr dom,
> +/*
> + * Attach a device given by XML, the change will be persistent
> + * and domain XML definition file is updated.
> + */
> +static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef,
> + virDomainDeviceDefPtr newdev)
> +{
> +
> + /* At first, check device confliction */
s/device confliction/for device support/
will fix.
> + switch(newdev->type) {
> + default:
> + qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("Sorry, the device is not suppored for now"));
s/suppored/supported/ (twice)
will fix. ....I'll use spell checker.
> +
> +static int qemuDomainModifyDevicePersistent(virDomainPtr dom,
> const char *xml,
Indentation is not consistent.
ok.
> - unsigned int flags) {
> - if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
> - qemuReportError(VIR_ERR_OPERATION_INVALID,
> - "%s", _("cannot modify the persistent
configuration of a domain"));
> + unsigned int attach, int flags)
> +{
> + struct qemud_driver *driver;
> + virDomainDeviceDefPtr device;
> + virDomainDefPtr vmdef;
> + virDomainObjPtr vm;
> + int ret = -1;
> +
> + if (!dom || !dom->conn || !dom->name || !xml) {
> + qemuReportError(VIR_ERR_INTERNAL_ERROR,
> + _("internal error : %s"), __FUNCTION__);
> + return -1;
> + }
> + /*
> + * When both of MODIFY_CONFIG and MODIFY_LIVE is specified at the same time,
> + * return error for now. We should support this later.
> + */
> + if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
> + qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("Now, cannot modify alive domain and its definition
"
> + "at the same time."));
It shouldn't be too hard to attempt supporting this now - after all, we
already did just that with Taku Izumi's patch for virsh setmem.
qemudDomainSetMemoryFlags can be a good reference point for how to
implement a command that first validates all conditions required by the
flags, then modifies the live configuration if requested and still live,
then modifies the persistent configuration if requested.
Can we do it in atomic ?
I think there have been enough findings already (Wen's feedback
has also
been useful), as well as quite a bit of upstream churn that will require
rebasing aspects of your patch, that I'll wait for v5 of this series; if
you use 'git send-email --subject-prefix=PATCHv5', that will make it
easier to recognize from the subject line that it is a resend.
ok, will do.
Thanks,
-Kame
8:28 p.m.
New subject: [libvirt] [PATCH 3/5] libvirt/qemu - support persistent modification of devices.
> > +
> > +static int qemuDomainModifyDevicePersistent(virDomainPtr dom,
> > const char *xml,
>
> Indentation is not consistent.
>
ok.
FYI, if you use vim as your editor, the following configuration can save
you a lot of time:
set autoindent
set smartindent
set cindent
set tabstop=4
set shiftwidth=4
set expandtab
set cinoptions=(0,:0,l1,t0
Just select the code snippet and press `=' will indent the code nicely.
--
Thanks,
Hu Tao
8:31 p.m.
New subject: [libvirt] [PATCH 3/5] libvirt/qemu - support persistent modification of devices.
On 03/15/2011 07:28 PM, Hu Tao wrote:
>>> +
>>> +static int qemuDomainModifyDevicePersistent(virDomainPtr dom,
>>> const char *xml,
>>
>> Indentation is not consistent.
>>
> ok.
FYI, if you use vim as your editor, the following configuration can save
you a lot of time:
set autoindent
set smartindent
set cindent
set tabstop=4
set shiftwidth=4
set expandtab
set cinoptions=(0,:0,l1,t0
Just select the code snippet and press `=' will indent the code nicely.
We already have .dir-locals.el to auto-set indentation for emacs users;
does vim support a directory-local configuration file that would be
worth checking into libvirt.git to make vim auto-indent with consistent
rules for all files found in the libvirt directory tree? [If you
haven't guessed, I'm not a vim user]
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
9:25 p.m.
New subject: [libvirt] [PATCH 3/5] libvirt/qemu - support persistent modification of devices.
On Wed, 16 Mar 2011 09:28:46 +0800
Hu Tao <hutao(a)cn.fujitsu.com> wrote:
> > > +
> > > +static int qemuDomainModifyDevicePersistent(virDomainPtr dom,
> > > const char *xml,
> >
> > Indentation is not consistent.
> >
> ok.
FYI, if you use vim as your editor, the following configuration can save
you a lot of time:
set autoindent
set smartindent
set cindent
set tabstop=4
set shiftwidth=4
set expandtab
set cinoptions=(0,:0,l1,t0
Just select the code snippet and press `=' will indent the code nicely.
Thank you. it's helpful.
-Kame
--
Thanks,
Hu Tao
7:12 p.m.
New subject: [libvirt] [PATCH 4/5] libvirt/qemu - support modification of disks in inactive domain
From 8d2544bc773a2222c8aa1fdfc5cade20d8c1e958 Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
Date: Thu, 3 Mar 2011 09:53:08 +0900
Subject: [PATCH 4/5] libvirt/qemu - support modification of disks in inactive domain
This patch adds support for persistent modification of disks in
inactive domain.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
---
src/qemu/qemu_driver.c | 42 ++++++++++++++++++++++++++++++++++++++++++
1 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3248cdc..4df6bf9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4099,6 +4099,18 @@ cleanup:
return ret;
}
+static int qemuDomainFindDiskByName(virDomainDefPtr vmdef, const char *name)
+{
+ virDomainDiskDefPtr vdisk;
+ int i;
+
+ for (i = 0; i < vmdef->ndisks; i++) {
+ vdisk = vmdef->disks[i];
+ if (STREQ(vdisk->dst, name))
+ return i;
+ }
+ return -1;
+}
/*
* Attach a device given by XML, the change will be persistent
* and domain XML definition file is updated.
@@ -4106,9 +4118,26 @@ cleanup:
static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef,
virDomainDeviceDefPtr newdev)
{
+ virDomainDiskDefPtr disk;
/* At first, check device confliction */
switch(newdev->type) {
+ case VIR_DOMAIN_DEVICE_DISK:
+ disk = newdev->data.disk;
+ if (qemuDomainFindDiskByName(vmdef, disk->dst) >= 0) {
+ qemuReportError(VIR_ERR_INVALID_ARG,
+ _("target %s already exists."), disk->dst);
+ return -1;
+ }
+
+ if (virDomainDiskInsert(vmdef, disk)) /* only failed with OOM */
+ return -1;
+
+ if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO &&
+ qemuDomainAssignPCIAddresses(vmdef) < 0)
+ return -1;
+ newdev->data.disk = NULL;
+ break;
default:
qemuReportError(VIR_ERR_INVALID_ARG, "%s",
_("Sorry, the device is not suppored for now"));
@@ -4121,7 +4150,20 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef,
static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef,
virDomainDeviceDefPtr device)
{
+ int x;
+ virDomainDiskDefPtr disk;
+
switch(device->type) {
+ case VIR_DOMAIN_DEVICE_DISK:
+ disk = device->data.disk;
+ x = qemuDomainFindDiskByName(vmdef, disk->dst);
+ if (x < 0) {
+ qemuReportError(VIR_ERR_INVALID_ARG,
+ _("target %s doesn't exist."), disk->dst);
+ return -1;
+ }
+ virDomainDiskRemove(vmdef, x);
+ break;
default:
qemuReportError(VIR_ERR_INVALID_ARG, "%s",
_("Sorry, the device is not suppored for now"));
--
1.7.4.1
4:31 a.m.
New subject: [libvirt] [PATCH 4/5] libvirt/qemu - support modification of disks in inactive domain
At 03/03/2011 09:12 AM, KAMEZAWA Hiroyuki Write:
>From 8d2544bc773a2222c8aa1fdfc5cade20d8c1e958 Mon Sep 17 00:00:00
2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
Date: Thu, 3 Mar 2011 09:53:08 +0900
Subject: [PATCH 4/5] libvirt/qemu - support modification of disks in inactive domain
This patch adds support for persistent modification of disks in
inactive domain.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
---
src/qemu/qemu_driver.c | 42 ++++++++++++++++++++++++++++++++++++++++++
1 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3248cdc..4df6bf9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4099,6 +4099,18 @@ cleanup:
return ret;
}
+static int qemuDomainFindDiskByName(virDomainDefPtr vmdef, const char *name)
+{
+ virDomainDiskDefPtr vdisk;
+ int i;
+
+ for (i = 0; i < vmdef->ndisks; i++) {
+ vdisk = vmdef->disks[i];
+ if (STREQ(vdisk->dst, name))
+ return i;
+ }
+ return -1;
+}
This function can be moved into src/conf/domain_conf.c, and it may be used by other
driver if this driver supports attach/detach device into inactive domain.
/*
* Attach a device given by XML, the change will be persistent
* and domain XML definition file is updated.
@@ -4106,9 +4118,26 @@ cleanup:
static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef,
virDomainDeviceDefPtr newdev)
{
+ virDomainDiskDefPtr disk;
/* At first, check device confliction */
switch(newdev->type) {
+ case VIR_DOMAIN_DEVICE_DISK:
+ disk = newdev->data.disk;
+ if (qemuDomainFindDiskByName(vmdef, disk->dst) >= 0) {
+ qemuReportError(VIR_ERR_INVALID_ARG,
+ _("target %s already exists."), disk->dst);
+ return -1;
+ }
+
+ if (virDomainDiskInsert(vmdef, disk)) /* only failed with OOM */
+ return -1;
+
+ if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO &&
+ qemuDomainAssignPCIAddresses(vmdef) < 0)
+ return -1;
Use virDomainDefAddImplicitControllers() to add any implied controllers
which aren't present and used by attached disk.
+ newdev->data.disk = NULL;
+ break;
default:
qemuReportError(VIR_ERR_INVALID_ARG, "%s",
_("Sorry, the device is not suppored for now"));
@@ -4121,7 +4150,20 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr
vmdef,
static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef,
virDomainDeviceDefPtr device)
{
+ int x;
+ virDomainDiskDefPtr disk;
+
switch(device->type) {
+ case VIR_DOMAIN_DEVICE_DISK:
+ disk = device->data.disk;
+ x = qemuDomainFindDiskByName(vmdef, disk->dst);
+ if (x < 0) {
+ qemuReportError(VIR_ERR_INVALID_ARG,
+ _("target %s doesn't exist."), disk->dst);
+ return -1;
+ }
+ virDomainDiskRemove(vmdef, x);
+ break;
default:
qemuReportError(VIR_ERR_INVALID_ARG, "%s",
_("Sorry, the device is not suppored for now"));
8:59 p.m.
New subject: [libvirt] [PATCH 4/5] libvirt/qemu - support modification of disks in inactive domain
On Mon, 14 Mar 2011 17:31:49 +0800
Wen Congyang <wency(a)cn.fujitsu.com> wrote:
At 03/03/2011 09:12 AM, KAMEZAWA Hiroyuki Write:
>>From 8d2544bc773a2222c8aa1fdfc5cade20d8c1e958 Mon Sep 17 00:00:00 2001
> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
> Date: Thu, 3 Mar 2011 09:53:08 +0900
> Subject: [PATCH 4/5] libvirt/qemu - support modification of disks in inactive
domain
>
> This patch adds support for persistent modification of disks in
> inactive domain.
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
> ---
> src/qemu/qemu_driver.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 42 insertions(+), 0 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 3248cdc..4df6bf9 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4099,6 +4099,18 @@ cleanup:
> return ret;
> }
>
> +static int qemuDomainFindDiskByName(virDomainDefPtr vmdef, const char *name)
> +{
> + virDomainDiskDefPtr vdisk;
> + int i;
> +
> + for (i = 0; i < vmdef->ndisks; i++) {
> + vdisk = vmdef->disks[i];
> + if (STREQ(vdisk->dst, name))
> + return i;
> + }
> + return -1;
> +}
This function can be moved into src/conf/domain_conf.c, and it may be used by other
driver if this driver supports attach/detach device into inactive domain.
ok, will move.
> /*
> * Attach a device given by XML, the change will be persistent
> * and domain XML definition file is updated.
> @@ -4106,9 +4118,26 @@ cleanup:
> static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef,
> virDomainDeviceDefPtr newdev)
> {
> + virDomainDiskDefPtr disk;
>
> /* At first, check device confliction */
> switch(newdev->type) {
> + case VIR_DOMAIN_DEVICE_DISK:
> + disk = newdev->data.disk;
> + if (qemuDomainFindDiskByName(vmdef, disk->dst) >= 0) {
> + qemuReportError(VIR_ERR_INVALID_ARG,
> + _("target %s already exists."), disk->dst);
> + return -1;
> + }
> +
> + if (virDomainDiskInsert(vmdef, disk)) /* only failed with OOM */
> + return -1;
> +
> + if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO &&
> + qemuDomainAssignPCIAddresses(vmdef) < 0)
> + return -1;
Use virDomainDefAddImplicitControllers() to add any implied controllers
which aren't present and used by attached disk.
For ide or scsi ? ok.
> + newdev->data.disk = NULL;
> + break;
> default:
> qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> _("Sorry, the device is not suppored for now"));
> @@ -4121,7 +4150,20 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr
vmdef,
> static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef,
> virDomainDeviceDefPtr device)
> {
> + int x;
> + virDomainDiskDefPtr disk;
> +
> switch(device->type) {
> + case VIR_DOMAIN_DEVICE_DISK:
> + disk = device->data.disk;
> + x = qemuDomainFindDiskByName(vmdef, disk->dst);
> + if (x < 0) {
> + qemuReportError(VIR_ERR_INVALID_ARG,
> + _("target %s doesn't exist."), disk->dst);
> + return -1;
> + }
> + virDomainDiskRemove(vmdef, x);
> + break;
> default:
> qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> _("Sorry, the device is not suppored for now"));
Thanks,
-Kame
7:13 p.m.
New subject: [libvirt] [PATCH 5/5] libvirt/qemu - allow modification nics of inactive domain
From 803cef98bc31a090b876f1774648e317401c806a Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
Date: Thu, 3 Mar 2011 10:01:39 +0900
Subject: [PATCH 5/5] libvirt/qemu - allow modification nics of inactive domain
This patch allows --persistent modification of nics with inactive
domains in qemu.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com>
---
src/conf/domain_conf.c | 24 +++++++++++++++++++++
src/conf/domain_conf.h | 3 ++
src/libvirt_private.syms | 2 +
src/qemu/qemu_driver.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 80 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4730683..8ef143c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4928,6 +4928,30 @@ void virDomainDiskRemove(virDomainDefPtr def, size_t i)
}
}
+int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net)
+{
+ if (VIR_REALLOC_N(def->nets, def->nnets) < 0)
+ return -1;
+ def->nets[def->nnets] = net;
+ def->nnets++;
+ return 0;
+}
+
+void virDomainNetRemove(virDomainDefPtr def, size_t i)
+{
+ if (def->nnets > 1) {
+ memmove(def->nets + i,
+ def->nets + i + 1,
+ sizeof(*def->nets) * (def->nnets - (i + 1)));
+ def->nnets--;
+ if (VIR_REALLOC_N(def->nets, def->nnets) < 0) {
+ /* ignore harmless */
+ }
+ } else {
+ VIR_FREE(def->nets);
+ def->nnets = 0;
+ }
+}
int virDomainControllerInsert(virDomainDefPtr def,
virDomainControllerDefPtr controller)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 30aeccc..d7d973e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1270,6 +1270,9 @@ int virDomainDiskDefAssignAddress(virCapsPtr caps,
virDomainDiskDefPtr def);
void virDomainDiskRemove(virDomainDefPtr def, size_t i);
+int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net);
+void virDomainNetRemove(virDomainDefPtr def, size_t i);
+
int virDomainControllerInsert(virDomainDefPtr def,
virDomainControllerDefPtr controller);
void virDomainControllerInsertPreAlloced(virDomainDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5e63a12..35b4e30 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -249,6 +249,8 @@ virDomainDiskIoTypeToString;
virDomainDiskRemove;
virDomainDiskTypeFromString;
virDomainDiskTypeToString;
+virDomainNetInsert;
+virDomainNetRemove;
virDomainFSDefFree;
virDomainFindByID;
virDomainFindByName;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4df6bf9..9f14880 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4111,6 +4111,21 @@ static int qemuDomainFindDiskByName(virDomainDefPtr vmdef, const
char *name)
}
return -1;
}
+
+static int qemuDomainFindNetByName(virDomainDefPtr vmdef,
+ const unsigned char *mac)
+{
+ virDomainNetDefPtr net;
+ int i;
+
+ for (i = 0; i < vmdef->nnets; i++) {
+ net = vmdef->nets[i];
+ /* For now, only MAC can be the key */
+ if (STREQ((char*)net->mac, (char*)mac))
+ return i;
+ }
+ return -1;
+}
/*
* Attach a device given by XML, the change will be persistent
* and domain XML definition file is updated.
@@ -4119,6 +4134,7 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef,
virDomainDeviceDefPtr newdev)
{
virDomainDiskDefPtr disk;
+ virDomainNetDefPtr net;
/* At first, check device confliction */
switch(newdev->type) {
@@ -4138,6 +4154,23 @@ static int qemuDomainAttachDevicePersistent(virDomainDefPtr vmdef,
return -1;
newdev->data.disk = NULL;
break;
+ case VIR_DOMAIN_DEVICE_NET:
+ net = newdev->data.net;
+ if (qemuDomainFindNetByName(vmdef, net->mac) >= 0) {
+ qemuReportError(VIR_ERR_INVALID_ARG,
+ _("target %s already exists."), net->mac);
+ return -1;
+ }
+
+ if (virDomainNetInsert(vmdef, net)) {
+ virReportOOMError();
+ return -1;
+ }
+ /* always PCI ? */
+ if (qemuDomainAssignPCIAddresses(vmdef) < 0)
+ return -1;
+ newdev->data.net = NULL;
+ break;
default:
qemuReportError(VIR_ERR_INVALID_ARG, "%s",
_("Sorry, the device is not suppored for now"));
@@ -4152,6 +4185,7 @@ static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef,
{
int x;
virDomainDiskDefPtr disk;
+ virDomainNetDefPtr net;
switch(device->type) {
case VIR_DOMAIN_DEVICE_DISK:
@@ -4164,6 +4198,23 @@ static int qemuDomainDetachDevicePersistent(virDomainDefPtr vmdef,
}
virDomainDiskRemove(vmdef, x);
break;
+ case VIR_DOMAIN_DEVICE_NET:
+ net = device->data.net;
+ /* need to find mac address */
+ if (net->ifname == NULL && strlen((char*)net->mac) == 0) {
+ qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("interface mac or name must be specified."));
+ return -1;
+ }
+ net = device->data.net;
+ x = qemuDomainFindNetByName(vmdef, net->mac);
+ if (x < 0) {
+ qemuReportError(VIR_ERR_INVALID_ARG,
+ _("interface mac: %s doesn't exist."),
net->mac);
+ return -1;
+ }
+ virDomainNetRemove(vmdef, x);
+ break;
default:
qemuReportError(VIR_ERR_INVALID_ARG, "%s",
_("Sorry, the device is not suppored for now"));
--
1.7.4.1
5:40 p.m.
On Thu, 3 Mar 2011 10:07:40 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com> wrote:
Now, Xen supports at(de)tach-disk, at(de)tach-interface for inactive domains
with --perisitent option. (Using --persisten against live domain will
cause error.)
Now, virsh attach-xxx/detach-xxx cannot modify inactive domains in qemu and
--persistent option just returns error.
This series adds support for modification of inactive domain's disk, interfaces.
Short description
1 ... clean up for Xen's driver
2 ... add my name to AUTHORS
3 ... add --persistent modification support, just a wrapper function.
4 ... add disk support
5 ... add nics support
What's bad with this patches ?
If libvirt never supports modification of XML in virsh *** command,
I'll change my direction and send patches to virtinst.
Thanks,
-Kame
6:06 p.m.
On 03/03/2011 04:40 PM, KAMEZAWA Hiroyuki wrote:
On Thu, 3 Mar 2011 10:07:40 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com> wrote:
>
> Now, Xen supports at(de)tach-disk, at(de)tach-interface for inactive domains
> with --perisitent option. (Using --persisten against live domain will
> cause error.)
>
> Now, virsh attach-xxx/detach-xxx cannot modify inactive domains in qemu and
> --persistent option just returns error.
>
> This series adds support for modification of inactive domain's disk, interfaces.
>
> Short description
> 1 ... clean up for Xen's driver
> 2 ... add my name to AUTHORS
> 3 ... add --persistent modification support, just a wrapper function.
> 4 ... add disk support
> 5 ... add nics support
>
What's bad with this patches ?
Personally, I think they are a good addition, but I haven't yet had time
to review them closely for potential coding gotchas. Rest assured, it's
on my todo list, and either these patches, or a further refined version,
will be included in the next libvirt release.
If libvirt never supports modification of XML in virsh *** command,
I'll change my direction and send patches to virtinst.
Sorry for the delays; it's been a busy week (I've been swamped trying to
debug libvirt crashes, which takes a higher priority on my plate over
libvirt feature additions). Don't give up yet; and it's fine to ping us
every few days if we continue to drag our feet!
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
6:04 p.m.
On Thu, 03 Mar 2011 17:06:05 -0700
Eric Blake <eblake(a)redhat.com> wrote:
On 03/03/2011 04:40 PM, KAMEZAWA Hiroyuki wrote:
> On Thu, 3 Mar 2011 10:07:40 +0900
> KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com> wrote:
>
>>
>> Now, Xen supports at(de)tach-disk, at(de)tach-interface for inactive domains
>> with --perisitent option. (Using --persisten against live domain will
>> cause error.)
>>
>> Now, virsh attach-xxx/detach-xxx cannot modify inactive domains in qemu and
>> --persistent option just returns error.
>>
>> This series adds support for modification of inactive domain's disk,
interfaces.
>>
>> Short description
>> 1 ... clean up for Xen's driver
>> 2 ... add my name to AUTHORS
>> 3 ... add --persistent modification support, just a wrapper function.
>> 4 ... add disk support
>> 5 ... add nics support
>>
>
> What's bad with this patches ?
Personally, I think they are a good addition, but I haven't yet had time
to review them closely for potential coding gotchas. Rest assured, it's
on my todo list, and either these patches, or a further refined version,
will be included in the next libvirt release.
>
> If libvirt never supports modification of XML in virsh *** command,
> I'll change my direction and send patches to virtinst.
Sorry for the delays; it's been a busy week (I've been swamped trying to
debug libvirt crashes, which takes a higher priority on my plate over
libvirt feature additions). Don't give up yet; and it's fine to ping us
every few days if we continue to drag our feet!
Okay, thank you for reply. I should be more patient.
BTW, what's crash ?
Sorry,
-Kame
6:28 p.m.
On 03/03/2011 05:04 PM, KAMEZAWA Hiroyuki wrote:
> Sorry for the delays; it's been a busy week (I've been
swamped trying to
> debug libvirt crashes, which takes a higher priority on my plate over
> libvirt feature additions). Don't give up yet; and it's fine to ping us
> every few days if we continue to drag our feet!
>
Okay, thank you for reply. I should be more patient.
BTW, what's crash ?
Transient domains and improper locking:
https://www.redhat.com/archives/libvir-list/2011-March/msg00112.html
Migration failures due to a flaw in qemu exec:command:
https://www.redhat.com/archives/libvir-list/2011-March/msg00040.html
I'm not the only one that can review and push patches, but I tend to be
one of the more active maintainers; thus when I get swamped debugging a
tough issue, sometimes that shows in my email response time and
subsequent level of community patches pushed into libvirt.git. As long
as you don't take it personally :)
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
5001
days inactive
5014
days old
28 comments
4 participants
participants (4)
-
Eric Blake -
Hu Tao -
KAMEZAWA Hiroyuki -
Wen Congyang