On 2012年02月28日 02:41, Eric Blake wrote:
On 02/27/2012 05:11 AM, Osier Yang wrote:
One could use it to eject, insert, or update media of the CDROM
or floppy drive. See the documents for more details.
s/documents/documentation/
---
tools/virsh.c | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/virsh.pod | 33 ++++++++++++-
2 files changed, 172 insertions(+), 3 deletions(-)
+static const vshCmdOptDef opts_change_media[] = {
+ {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
+ {"path", VSH_OT_DATA, VSH_OFLAG_REQ, N_("Fully-qualified path or "
+ "target of disk device")},
+ {"source", VSH_OT_DATA, 0, N_("source of the media")},
+ {"eject", VSH_OT_BOOL, 0, N_("Eject the media")},
+ {"insert", VSH_OT_BOOL, 0, N_("Insert the media")},
+ {"update", VSH_OT_BOOL, 0, N_("Update the media")},
I still wonder if --mode={eject|insert|update} would have been any
easier to code, but that's just painting the bikeshed, so don't worry
about it.
+static bool
+cmdChangeMedia(vshControl *ctl, const vshCmd *cmd)
+{
+ virDomainPtr dom = NULL;
+ const char *source = NULL;
+ const char *path = NULL;
+ const char *doc = NULL;
+ xmlNodePtr disk_node = NULL;
+ const char *disk_xml = NULL;
+ int flags = 0;
+ int config, live, current, force = 0;
s/int/bool/
+ int eject, insert, update = 0;
s/int/bool/
+ bool ret = false;
+ int prepare_type = 0;
+ const char *action = NULL;
+
+ config = vshCommandOptBool(cmd, "config");
+ live = vshCommandOptBool(cmd, "live");
+ current = vshCommandOptBool(cmd, "current");
+ force = vshCommandOptBool(cmd, "force");
+ eject = vshCommandOptBool(cmd, "eject");
+ insert = vshCommandOptBool(cmd, "insert");
+ update = vshCommandOptBool(cmd, "update");
Particularly since you are assigning all 7 variables from vshCommandOptBool.
+
+ if ((eject&& insert) ||
+ (insert&& update) ||
+ (eject&& update)) {
A shorter way to write this:
if (eject + insert + update> 1) {
Nice.
+ vshError(ctl, "%s", _("--eject, --insert, and --update must be specified "
+ "exclusively."));
+ return false;
+ }
+
+ if (eject) {
+ prepare_type = VSH_PREPARE_DISK_XML_EJECT;
+ action = "eject";
+ }
+
+ if (insert) {
+ prepare_type = VSH_PREPARE_DISK_XML_INSERT;
+ action = "insert";
+ }
+
+ if (update) {
+ prepare_type = VSH_PREPARE_DISK_XML_UPDATE;
+ action = "update";
+ }
+
+ /* Defaults to "update" */
+ if (!eject&& !insert&& !update) {
+ prepare_type = VSH_PREPARE_DISK_XML_UPDATE;
+ action = "update";
You can combine these two clauses:
if (update || (!eject&& !insert)) {
Nice.
+
+ if (virDomainUpdateDeviceFlags(dom, disk_xml, flags) != 0) {
+ vshError(ctl, _("Failed to %s media"), action);
Translators won't like this; there are languages where the spelling of
the rest of the sentence depends on the gender of the word in 'action'.
Better would be:
"Failed to complete '%s' action on media"
+ goto cleanup;
+ }
+
+ vshPrint(ctl, _("succeeded to %s media\n"), action);
and again, better would be:
"Successfully completed '%s' action on media'
@@ -16705,6 +16846,7 @@ static const vshCmdDef domManagementCmds[] = {
#ifndef WIN32
{"console", cmdConsole, opts_console, info_console, 0},
#endif
+ {"change-media", cmdChangeMedia, opts_change_media, info_change_media, 0},
Sorting - change-media comes _before_ console
+++ b/tools/virsh.pod
@@ -1490,9 +1490,10 @@ from the domain.
=item B<detach-interface> I<domain-id> I<type> [I<--mac mac>]
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.
+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.
Unrelated hunk; for backport purposes, it would be nicer if you split
this cleanup hunk into a separate (pre-approved) patch.
Ah, yeah, right.
=item B<update-device> I<domain-id> I<file> [I<--persistent>] [I<--force>]
@@ -1503,6 +1504,32 @@ 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 to learn about libvirt
XML format for a device.
+=item B<change-media> I<domain-id> I<path> [I<--eject>] [I<--insert>]
+[I<--update>] [I<source>] [I<--force>] [I<--current>] [I<--live>] [I<--config>]
Per convention on other commands, --current is mutually exclusive with
--live or --config, so I'd show that part of the line as:
[[I<--live>] [I<--config>] | [I<--current>]]
ACK with nits fixed, and about time we had this useful command! (I
tried, and failed to complete, something similar more than a year ago.)
Fixed the nits and pushed, thanks for the reviewing!
Osier
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list