4:22 p.m.
A respin of the series that changes some virsh commands to handle flags more
sanely. This version fixes comments by Martin, Laine and Eric.
This series omits already acked patches.
Peter Krempa (4):
virsh: Fix semantics of --config for "update-device" command
virsh-domain: Add --live, --config, --current logic to
cmdDetachInterface
virsh-domain: Add --live, --config, --current logic to cmdDetachDevice
virsh-domain: Add --live, --config, --current logic to cmdDetachDisk
tools/virsh-domain.c | 182 +++++++++++++++++++++++++++++++++++++--------------
tools/virsh.pod | 81 +++++++++++++++++------
2 files changed, 194 insertions(+), 69 deletions(-)
--
1.8.1.5
4:22 p.m.
New subject: [libvirt] [PATCHv2 1/4] virsh: Fix semantics of --config for "update-device" command
The man page states that with --config the next boot is affected. This
can be understood as if _only_ the next boot was affected. This isn't
true if the machine is running.
This patch adds the full --live, --config, --current infrastructure and
tweaks stuff to correctly support the obsolete --persistent flag.
---
Notes:
Version 2:
- note in the docs that semantics of the flags were fixed
tools/virsh-domain.c | 40 +++++++++++++++++++++++++++++-----------
tools/virsh.pod | 24 +++++++++++++++++-------
2 files changed, 46 insertions(+), 18 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index fa5ab3d..2a228f5 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9246,13 +9246,21 @@ static const vshCmdOptDef opts_update_device[] = {
.help = N_("XML file")
},
{.name = "persistent",
- .type = VSH_OT_ALIAS,
- .help = "config"
+ .type = VSH_OT_BOOL,
+ .help = N_("make live change persistent")
},
{.name = "config",
.type = VSH_OT_BOOL,
.help = N_("affect next boot")
},
+ {.name = "live",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect running domain")
+ },
+ {.name = "current",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect current domain")
+ },
{.name = "force",
.type = VSH_OT_BOOL,
.help = N_("force device update")
@@ -9267,7 +9275,21 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
const char *from = NULL;
char *buffer = NULL;
bool ret = false;
- unsigned int flags;
+ unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+ bool current = vshCommandOptBool(cmd, "current");
+ bool config = vshCommandOptBool(cmd, "config");
+ bool live = vshCommandOptBool(cmd, "live");
+ bool persistent = vshCommandOptBool(cmd, "persistent");
+
+ VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
+
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (config || persistent)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -9275,19 +9297,15 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
goto cleanup;
+ if (persistent &&
+ virDomainIsActive(dom) == 1)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) {
vshReportError(ctl);
goto cleanup;
}
- if (vshCommandOptBool(cmd, "config")) {
- flags = VIR_DOMAIN_AFFECT_CONFIG;
- if (virDomainIsActive(dom) == 1)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- } else {
- flags = VIR_DOMAIN_AFFECT_LIVE;
- }
-
if (vshCommandOptBool(cmd, "force"))
flags |= VIR_DOMAIN_DEVICE_MODIFY_FORCE;
diff --git a/tools/virsh.pod b/tools/virsh.pod
index e7e82e3..04ea542 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1888,18 +1888,28 @@ If I<--config> is specified, alter persistent configuration,
effect observed
on next boot, for compatibility purposes, I<--persistent> is alias of
I<--config>.
-=item B<update-device> I<domain> I<file> [I<--config>]
[I<--force>]
+=item B<update-device> I<domain> I<file> [I<--force>]
+[[[I<--live>] [I<--config>] | [I<--current>]] |
[I<--persistent>]]
Update the characteristics of a device associated with I<domain>,
-based on the device definition in an XML I<file>. If the I<--config>
-option is used, the changes will take affect the next time libvirt
-starts the domain. For compatibility purposes, I<--persistent> is
-alias of I<--config>. The I<--force> option can be used to force
-device update, e.g., to eject a CD-ROM even if it is locked/mounted in
-the domain. See the documentation at
+based on the device definition in an XML I<file>. The I<--force> option
+can be used to force device update, e.g., to eject a CD-ROM even if it is
+locked/mounted in the domain. See the documentation at
L<http://libvirt.org/formatdomain.html#elementsDevices> to learn about
libvirt XML format for a device.
+If I<--live> is specified, affect a running domain.
+If I<--config> is specified, affect the next startup of a persistent domain.
+If I<--current> is specified, affect the current domain state.
+Both I<--live> and I<--config> flags may be given, but I<--current> is
+exclusive. Not specifying any flag is the same as specifying I<--current>.
+
+For compatibility purposes, I<--persistent> behaves like I<--config> for
+an offline domain, and like I<--live> I<--config> for a running domain.
+
+Note that older versions of virsh used I<--config> as an alias for
+I<--persistent>.
+
=item B<change-media> I<domain> I<path> [I<--eject>]
[I<--insert>]
[I<--update>] [I<source>] [I<--force>] [[I<--live>]
[I<--config>] | [I<--current>]]
--
1.8.1.5
1:41 p.m.
New subject: [libvirt] [PATCHv2 1/4] virsh: Fix semantics of --config for "update-device" command
On 03/31/2013 03:22 PM, Peter Krempa wrote:
The man page states that with --config the next boot is affected.
This
can be understood as if _only_ the next boot was affected. This isn't
true if the machine is running.
This patch adds the full --live, --config, --current infrastructure and
tweaks stuff to correctly support the obsolete --persistent flag.
---
Notes:
Version 2:
- note in the docs that semantics of the flags were fixed
tools/virsh-domain.c | 40 +++++++++++++++++++++++++++++-----------
tools/virsh.pod | 24 +++++++++++++++++-------
2 files changed, 46 insertions(+), 18 deletions(-)
ACK
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:46 p.m.
New subject: [libvirt] [PATCHv2 1/4] virsh: Fix semantics of --config for "update-device" command
On 03/31/2013 05:22 PM, Peter Krempa wrote:
The man page states that with --config the next boot is affected.
This
can be understood as if _only_ the next boot was affected. This isn't
true if the machine is running.
You should probably change this comment to make it clear that you're
changing the behavior of the option, rather than the documentation of
what it does.
After our discussion last week, I do agree that, although this is a
change in behavior of an already-released command, it's acceptable
because 1) it was different behavior from all other commands using
--config, and 2) it was documented as behaving as all other commands.
This patch adds the full --live, --config, --current infrastructure and
tweaks stuff to correctly support the obsolete --persistent flag.
---
Notes:
Version 2:
- note in the docs that semantics of the flags were fixed
ACK once you note in the commit log that the code was fixed, not the
documentation.
tools/virsh-domain.c | 40 +++++++++++++++++++++++++++++-----------
tools/virsh.pod | 24 +++++++++++++++++-------
2 files changed, 46 insertions(+), 18 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index fa5ab3d..2a228f5 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9246,13 +9246,21 @@ static const vshCmdOptDef opts_update_device[] = {
.help = N_("XML file")
},
{.name = "persistent",
- .type = VSH_OT_ALIAS,
- .help = "config"
+ .type = VSH_OT_BOOL,
+ .help = N_("make live change persistent")
},
{.name = "config",
.type = VSH_OT_BOOL,
.help = N_("affect next boot")
},
+ {.name = "live",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect running domain")
+ },
+ {.name = "current",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect current domain")
+ },
{.name = "force",
.type = VSH_OT_BOOL,
.help = N_("force device update")
@@ -9267,7 +9275,21 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
const char *from = NULL;
char *buffer = NULL;
bool ret = false;
- unsigned int flags;
+ unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+ bool current = vshCommandOptBool(cmd, "current");
+ bool config = vshCommandOptBool(cmd, "config");
+ bool live = vshCommandOptBool(cmd, "live");
+ bool persistent = vshCommandOptBool(cmd, "persistent");
+
+ VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
+
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (config || persistent)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -9275,19 +9297,15 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
goto cleanup;
+ if (persistent &&
+ virDomainIsActive(dom) == 1)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+
if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) {
vshReportError(ctl);
goto cleanup;
}
- if (vshCommandOptBool(cmd, "config")) {
- flags = VIR_DOMAIN_AFFECT_CONFIG;
- if (virDomainIsActive(dom) == 1)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- } else {
- flags = VIR_DOMAIN_AFFECT_LIVE;
- }
-
if (vshCommandOptBool(cmd, "force"))
flags |= VIR_DOMAIN_DEVICE_MODIFY_FORCE;
diff --git a/tools/virsh.pod b/tools/virsh.pod
index e7e82e3..04ea542 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1888,18 +1888,28 @@ If I<--config> is specified, alter persistent
configuration, effect observed
on next boot, for compatibility purposes, I<--persistent> is alias of
I<--config>.
-=item B<update-device> I<domain> I<file> [I<--config>]
[I<--force>]
+=item B<update-device> I<domain> I<file> [I<--force>]
+[[[I<--live>] [I<--config>] | [I<--current>]] |
[I<--persistent>]]
Update the characteristics of a device associated with I<domain>,
-based on the device definition in an XML I<file>. If the I<--config>
-option is used, the changes will take affect the next time libvirt
-starts the domain. For compatibility purposes, I<--persistent> is
-alias of I<--config>. The I<--force> option can be used to force
-device update, e.g., to eject a CD-ROM even if it is locked/mounted in
-the domain. See the documentation at
+based on the device definition in an XML I<file>. The I<--force> option
+can be used to force device update, e.g., to eject a CD-ROM even if it is
+locked/mounted in the domain. See the documentation at
L<http://libvirt.org/formatdomain.html#elementsDevices> to learn about
libvirt XML format for a device.
+If I<--live> is specified, affect a running domain.
+If I<--config> is specified, affect the next startup of a persistent domain.
+If I<--current> is specified, affect the current domain state.
+Both I<--live> and I<--config> flags may be given, but I<--current>
is
+exclusive. Not specifying any flag is the same as specifying I<--current>.
+
+For compatibility purposes, I<--persistent> behaves like I<--config> for
+an offline domain, and like I<--live> I<--config> for a running domain.
+
+Note that older versions of virsh used I<--config> as an alias for
+I<--persistent>.
+
=item B<change-media> I<domain> I<path> [I<--eject>]
[I<--insert>]
[I<--update>] [I<source>] [I<--force>] [[I<--live>]
[I<--config>] | [I<--current>]]
9:32 a.m.
New subject: [libvirt] [PATCHv2 1/4] virsh: Fix semantics of --config for "update-device" command
On 04/01/13 21:46, Laine Stump wrote:
On 03/31/2013 05:22 PM, Peter Krempa wrote:
> The man page states that with --config the next boot is affected. This
> can be understood as if _only_ the next boot was affected. This isn't
> true if the machine is running.
You should probably change this comment to make it clear that you're
changing the behavior of the option, rather than the documentation of
what it does.
After our discussion last week, I do agree that, although this is a
change in behavior of an already-released command, it's acceptable
because 1) it was different behavior from all other commands using
--config, and 2) it was documented as behaving as all other commands.
>
> This patch adds the full --live, --config, --current infrastructure and
> tweaks stuff to correctly support the obsolete --persistent flag.
> ---
>
> Notes:
> Version 2:
> - note in the docs that semantics of the flags were fixed
ACK once you note in the commit log that the code was fixed, not the
documentation.
I mentioned this in the commit message, removed the stray newline in 3/4
and pushed this with the rest of the series.
Thanks.
Peter
4:22 p.m.
New subject: [libvirt] [PATCHv2 2/4] virsh-domain: Add --live, --config, --current logic to cmdDetachInterface
Use the established approach to improve this function too.
---
Notes:
Version 2:
- state that legacy API has no well defined semantics
- add note about possible semantics change
- remove mutual exclusivity of --persistent --live
- remove --force parameter added due to too much copy and paste
tools/virsh-domain.c | 47 ++++++++++++++++++++++++++++++++++-------------
tools/virsh.pod | 19 +++++++++++++++----
2 files changed, 49 insertions(+), 17 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 2a228f5..d7778dd 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9352,13 +9352,21 @@ static const vshCmdOptDef opts_detach_interface[] = {
.help = N_("MAC address")
},
{.name = "persistent",
- .type = VSH_OT_ALIAS,
- .help = "config"
+ .type = VSH_OT_BOOL,
+ .help = N_("make live change persistent")
},
{.name = "config",
.type = VSH_OT_BOOL,
.help = N_("affect next boot")
},
+ {.name = "live",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect running domain")
+ },
+ {.name = "current",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect current domain")
+ },
{.name = NULL}
};
@@ -9372,12 +9380,26 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
xmlNodePtr cur = NULL, matchNode = NULL;
xmlBufferPtr xml_buf = NULL;
const char *mac =NULL, *type = NULL;
- char *doc;
+ char *doc = NULL;
char buf[64];
int i = 0, diff_mac;
int ret;
int functionReturn = false;
- unsigned int flags;
+ unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+ bool current = vshCommandOptBool(cmd, "current");
+ bool config = vshCommandOptBool(cmd, "config");
+ bool live = vshCommandOptBool(cmd, "live");
+ bool persistent = vshCommandOptBool(cmd, "persistent");
+
+ VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
+
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (config || persistent)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -9388,13 +9410,14 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "mac", &mac) < 0)
goto cleanup;
- doc = virDomainGetXMLDesc(dom, 0);
- if (!doc)
+ if (persistent &&
+ virDomainIsActive(dom) == 1)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+ if (!(doc = virDomainGetXMLDesc(dom, 0)))
goto cleanup;
- xml = virXMLParseStringCtxt(doc, _("(domain_definition)"), &ctxt);
- VIR_FREE(doc);
- if (!xml) {
+ if (!(xml = virXMLParseStringCtxt(doc, _("(domain_definition)"),
&ctxt))) {
vshError(ctl, "%s", _("Failed to get interface
information"));
goto cleanup;
}
@@ -9459,10 +9482,7 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
- if (vshCommandOptBool(cmd, "config")) {
- flags = VIR_DOMAIN_AFFECT_CONFIG;
- if (virDomainIsActive(dom) == 1)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
+ if (flags != 0) {
ret = virDomainDetachDeviceFlags(dom,
(char *)xmlBufferContent(xml_buf),
flags);
@@ -9478,6 +9498,7 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
}
cleanup:
+ VIR_FREE(doc);
virDomainFree(dom);
xmlXPathFreeObject(obj);
xmlXPathFreeContext(ctxt);
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 04ea542..e67c3cc 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1877,16 +1877,27 @@ If I<--config> is specified, alter persistent configuration,
effect observed
on next boot, for compatibility purposes, I<--persistent> is alias of
I<--config>.
-=item B<detach-interface> I<domain> I<type> [I<--mac mac>]
[I<--config>]
+=item B<detach-interface> I<domain> I<type> [I<--mac mac>]
+[[[I<--live>] [I<--config>] | [I<--current>]] |
[I<--persistent>]]
Detach a network interface from a domain.
I<type> can be either I<network> to indicate a physical network device or
I<bridge> to indicate a bridge to a device. It is recommended to use the
I<mac> option to distinguish between the interfaces if more than one are
present on the domain.
-If I<--config> is specified, alter persistent configuration, effect observed
-on next boot, for compatibility purposes, I<--persistent> is alias of
-I<--config>.
+
+If I<--live> is specified, affect a running domain.
+If I<--config> is specified, affect the next startup of a persistent domain.
+If I<--current> is specified, affect the current domain state.
+Both I<--live> and I<--config> flags may be given, but I<--current> is
+exclusive. When no flag is specified legacy API is used whose behavior depends
+on the hypervisor driver.
+
+For compatibility purposes, I<--persistent> behaves like I<--config> for
+an offline domain, and like I<--live> I<--config> for a running domain.
+
+Note that older versions of virsh used I<--config> as an alias for
+I<--persistent>.
=item B<update-device> I<domain> I<file> [I<--force>]
[[[I<--live>] [I<--config>] | [I<--current>]] |
[I<--persistent>]]
--
1.8.1.5
1:42 p.m.
New subject: [libvirt] [PATCHv2 2/4] virsh-domain: Add --live, --config, --current logic to cmdDetachInterface
On 03/31/2013 03:22 PM, Peter Krempa wrote:
Use the established approach to improve this function too.
---
Notes:
Version 2:
- state that legacy API has no well defined semantics
- add note about possible semantics change
- remove mutual exclusivity of --persistent --live
- remove --force parameter added due to too much copy and paste
tools/virsh-domain.c | 47 ++++++++++++++++++++++++++++++++++-------------
tools/virsh.pod | 19 +++++++++++++++----
2 files changed, 49 insertions(+), 17 deletions(-)
ACK
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:22 p.m.
New subject: [libvirt] [PATCHv2 3/4] virsh-domain: Add --live, --config, --current logic to cmdDetachDevice
Use the established approach to improve this function too.
---
Notes:
Version 2:
- same changes as previous patch
tools/virsh-domain.c | 41 ++++++++++++++++++++++++++++++++---------
tools/virsh.pod | 19 +++++++++++++++----
2 files changed, 47 insertions(+), 13 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index d7778dd..fa9279b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9167,14 +9167,23 @@ static const vshCmdOptDef opts_detach_device[] = {
.help = N_("XML file")
},
{.name = "persistent",
- .type = VSH_OT_ALIAS,
- .help = "config"
+ .type = VSH_OT_BOOL,
+ .help = N_("make live change persistent")
},
{.name = "config",
.type = VSH_OT_BOOL,
.help = N_("affect next boot")
},
+ {.name = "live",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect running domain")
+ },
+ {.name = "current",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect current domain")
+ },
{.name = NULL}
+
};
static bool
@@ -9185,11 +9194,29 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd)
char *buffer = NULL;
int ret;
bool funcRet = false;
- unsigned int flags;
+ bool current = vshCommandOptBool(cmd, "current");
+ bool config = vshCommandOptBool(cmd, "config");
+ bool live = vshCommandOptBool(cmd, "live");
+ bool persistent = vshCommandOptBool(cmd, "persistent");
+ unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+
+ VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
+
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (config || persistent)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
+ if (persistent &&
+ virDomainIsActive(dom) == 1)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+
if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
goto cleanup;
@@ -9198,14 +9225,10 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
- if (vshCommandOptBool(cmd, "config")) {
- flags = VIR_DOMAIN_AFFECT_CONFIG;
- if (virDomainIsActive(dom) == 1)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
+ if (flags != 0)
ret = virDomainDetachDeviceFlags(dom, buffer, flags);
- } else {
+ else
ret = virDomainDetachDevice(dom, buffer);
- }
if (ret < 0) {
vshError(ctl, _("Failed to detach device from %s"), from);
diff --git a/tools/virsh.pod b/tools/virsh.pod
index e67c3cc..b3aad62 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1859,16 +1859,27 @@ B<Note>: the optional target value is the name of a device
to be created
as the back-end on the node. If not provided a device named "vnetN" or
"vifN"
will be created automatically.
-=item B<detach-device> I<domain> I<FILE> [I<--config>]
+=item B<detach-device> I<domain> I<FILE>
+[[[I<--live>] [I<--config>] | [I<--current>]] |
[I<--persistent>]]
Detach a device from the domain, takes the same kind of XML descriptions
as command B<attach-device>.
-If I<--config> is specified, alter persistent configuration, effect observed
-on next boot, for compatibility purposes, I<--persistent> is alias of
-I<--config>.
For passthrough host devices, see also B<nodedev-reattach>, needed if
the device does not use managed mode.
+If I<--live> is specified, affect a running domain.
+If I<--config> is specified, affect the next startup of a persistent domain.
+If I<--current> is specified, affect the current domain state.
+Both I<--live> and I<--config> flags may be given, but I<--current> is
+exclusive. When no flag is specified legacy API is used whose behavior depends
+on the hypervisor driver.
+
+For compatibility purposes, I<--persistent> behaves like I<--config> for
+an offline domain, and like I<--live> I<--config> for a running domain.
+
+Note that older versions of virsh used I<--config> as an alias for
+I<--persistent>.
+
=item B<detach-disk> I<domain> I<target> [I<--config>]
Detach a disk device from a domain. The I<target> is the device as seen
--
1.8.1.5
1:43 p.m.
New subject: [libvirt] [PATCHv2 3/4] virsh-domain: Add --live, --config, --current logic to cmdDetachDevice
On 03/31/2013 03:22 PM, Peter Krempa wrote:
Use the established approach to improve this function too.
---
Notes:
Version 2:
- same changes as previous patch
tools/virsh-domain.c | 41 ++++++++++++++++++++++++++++++++---------
tools/virsh.pod | 19 +++++++++++++++----
2 files changed, 47 insertions(+), 13 deletions(-)
ACK.
+ },
{.name = NULL}
+
};
Looks like this blank line is an accident.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:22 p.m.
New subject: [libvirt] [PATCHv2 4/4] virsh-domain: Add --live, --config, --current logic to cmdDetachDisk
Use the established approach to improve this function too.
---
Notes:
Version 2:
- same notes as previous patch
tools/virsh-domain.c | 54 +++++++++++++++++++++++++++++++++++-----------------
tools/virsh.pod | 19 ++++++++++++++----
2 files changed, 52 insertions(+), 21 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index fa9279b..1be1a26 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9771,13 +9771,21 @@ static const vshCmdOptDef opts_detach_disk[] = {
.help = N_("target of disk device")
},
{.name = "persistent",
- .type = VSH_OT_ALIAS,
- .help = "config"
+ .type = VSH_OT_BOOL,
+ .help = N_("make live change persistent")
},
{.name = "config",
.type = VSH_OT_BOOL,
.help = N_("affect next boot")
},
+ {.name = "live",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect running domain")
+ },
+ {.name = "current",
+ .type = VSH_OT_BOOL,
+ .help = N_("affect current domain")
+ },
{.name = NULL}
};
@@ -9790,8 +9798,22 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
char *doc = NULL;
int ret;
bool functionReturn = false;
- unsigned int flags;
xmlNodePtr disk_node = NULL;
+ bool current = vshCommandOptBool(cmd, "current");
+ bool config = vshCommandOptBool(cmd, "config");
+ bool live = vshCommandOptBool(cmd, "live");
+ bool persistent = vshCommandOptBool(cmd, "persistent");
+ unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+
+ VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
+
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (config || persistent)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -9799,10 +9821,13 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "target", &target) < 0)
goto cleanup;
- doc = virDomainGetXMLDesc(dom, 0);
- if (!doc)
+ if (!(doc = virDomainGetXMLDesc(dom, 0)))
goto cleanup;
+ if (persistent &&
+ virDomainIsActive(dom) == 1)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+
if (!(disk_node = vshFindDisk(doc, target, VSH_FIND_DISK_NORMAL)))
goto cleanup;
@@ -9810,24 +9835,19 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
VSH_PREPARE_DISK_XML_NONE)))
goto cleanup;
- if (vshCommandOptBool(cmd, "config")) {
- flags = VIR_DOMAIN_AFFECT_CONFIG;
- if (virDomainIsActive(dom) == 1)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- ret = virDomainDetachDeviceFlags(dom,
- disk_xml,
- flags);
- } else {
+ if (flags != 0)
+ ret = virDomainDetachDeviceFlags(dom, disk_xml, flags);
+ else
ret = virDomainDetachDevice(dom, disk_xml);
- }
if (ret != 0) {
vshError(ctl, "%s", _("Failed to detach disk"));
- } else {
- vshPrint(ctl, "%s", _("Disk detached successfully\n"));
- functionReturn = true;
+ goto cleanup;
}
+ vshPrint(ctl, "%s", _("Disk detached successfully\n"));
+ functionReturn = true;
+
cleanup:
xmlFreeNode(disk_node);
VIR_FREE(disk_xml);
diff --git a/tools/virsh.pod b/tools/virsh.pod
index b3aad62..6ef509b 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1880,13 +1880,24 @@ an offline domain, and like I<--live> I<--config> for
a running domain.
Note that older versions of virsh used I<--config> as an alias for
I<--persistent>.
-=item B<detach-disk> I<domain> I<target> [I<--config>]
+=item B<detach-disk> I<domain> I<target>
+[[[I<--live>] [I<--config>] | [I<--current>]] |
[I<--persistent>]]
Detach a disk device from a domain. The I<target> is the device as seen
from the domain.
-If I<--config> is specified, alter persistent configuration, effect observed
-on next boot, for compatibility purposes, I<--persistent> is alias of
-I<--config>.
+
+If I<--live> is specified, affect a running domain.
+If I<--config> is specified, affect the next startup of a persistent domain.
+If I<--current> is specified, affect the current domain state.
+Both I<--live> and I<--config> flags may be given, but I<--current> is
+exclusive. When no flag is specified legacy API is used whose behavior depends
+on the hypervisor driver.
+
+For compatibility purposes, I<--persistent> behaves like I<--config> for
+an offline domain, and like I<--live> I<--config> for a running domain.
+
+Note that older versions of virsh used I<--config> as an alias for
+I<--persistent>.
=item B<detach-interface> I<domain> I<type> [I<--mac mac>]
[[[I<--live>] [I<--config>] | [I<--current>]] |
[I<--persistent>]]
--
1.8.1.5
1:44 p.m.
New subject: [libvirt] [PATCHv2 4/4] virsh-domain: Add --live, --config, --current logic to cmdDetachDisk
On 03/31/2013 03:22 PM, Peter Krempa wrote:
Use the established approach to improve this function too.
---
Notes:
Version 2:
- same notes as previous patch
tools/virsh-domain.c | 54 +++++++++++++++++++++++++++++++++++-----------------
tools/virsh.pod | 19 ++++++++++++++----
2 files changed, 52 insertions(+), 21 deletions(-)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4252
days inactive
4254
days old
10 comments
3 participants
participants (3)
-
Eric Blake -
Laine Stump -
Peter Krempa