[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH v2] add --cache, --serial, --sharable and --pciaddress options(was Re: [PATCH 1/3] add --cache option for attach-disk)



On Tue, Jul 05, 2011 at 11:58:31AM -0600, Eric Blake wrote:
> On 07/05/2011 01:25 AM, Hu Tao wrote:
> > Add --cache option to allow user to specify cache mode of disk device
> > from virsh command line when attaching a disk device.
> > ---
> >  tools/virsh.c   |   10 ++++++++--
> >  tools/virsh.pod |    5 +++--
> >  2 files changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/virsh.c b/tools/virsh.c
> > index 9a189fd..cab5a35 100644
> > --- a/tools/virsh.c
> > +++ b/tools/virsh.c
> > @@ -10176,6 +10176,7 @@ static const vshCmdOptDef opts_attach_disk[] = {
> >      {"target",  VSH_OT_DATA, VSH_OFLAG_REQ, N_("target of disk device")},
> >      {"driver",    VSH_OT_STRING, 0, N_("driver of disk device")},
> >      {"subdriver", VSH_OT_STRING, 0, N_("subdriver of disk device")},
> > +    {"cache",     VSH_OT_STRING, 0, N_("cache mode of disk device")},
> 
> Nice.
> 
> >  
> > +    ignore_value(vshCommandOptString(cmd, "cache", &cache));
> 
> Not so nice.
> 
> --cache ''
> 
> will make vshCommandOptString return -1, because that usage is a virsh
> usage error and should be diagnosed as such up front, rather than
> accidentally passing cache='' through the XML to the libvirt API.

I found that in the case of --cache '' vshCommandOptString returns 0,
with cache(3rd parameter) unchanged. So can we safely ignore value or do
I miss something?

This v2 also adds another new option, --pciaddress.

>From 3e3473baf24f1b6ff014cc26f05271a2da87d12e Mon Sep 17 00:00:00 2001
From: Hu Tao <hutao cn fujitsu com>
Date: Tue, 12 Jul 2011 15:19:42 +0800
Subject: [PATCH] add --cache, --serial, --sharable and --pciaddress options
 for attach-disk

This add three options for virsh command attach-disk.

--cache option allows user to specify cache mode of disk device
from virsh command line when attaching a disk device.

--serial option allows user to specify serial string of disk device
from virsh command line when attaching a disk device.

--sharable option allows user to specify whether the disk device is
sharable between domains when attaching a disk device from virsh
command line.

--pciaddress option allows user to specify pci address of disk
controller when attaching a disk device.
---
 tools/virsh.c   |   78 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 tools/virsh.pod |   10 +++++-
 2 files changed, 84 insertions(+), 4 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 3cdf043..1fa3b98 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -10168,19 +10168,64 @@ static const vshCmdOptDef opts_attach_disk[] = {
     {"target",  VSH_OT_DATA, VSH_OFLAG_REQ, N_("target of disk device")},
     {"driver",    VSH_OT_STRING, 0, N_("driver of disk device")},
     {"subdriver", VSH_OT_STRING, 0, N_("subdriver of disk device")},
+    {"cache",     VSH_OT_STRING, 0, N_("cache mode of disk device")},
     {"type",    VSH_OT_STRING, 0, N_("target device type")},
     {"mode",    VSH_OT_STRING, 0, N_("mode of device reading and writing")},
     {"persistent", VSH_OT_BOOL, 0, N_("persist disk attachment")},
     {"sourcetype", VSH_OT_STRING, 0, N_("type of source (block|file)")},
+    {"serial", VSH_OT_STRING, 0, N_("serial of disk device")},
+    {"shareable", VSH_OT_BOOL, 0, N_("shareable between domains")},
+    {"pciaddress", VSH_OT_STRING, 0, N_("pci address")},
     {NULL, 0, 0, NULL}
 };
 
+struct PCIAddress {
+    unsigned int domain;
+    unsigned int bus;
+    unsigned int slot;
+    unsigned int function;
+};
+
+
+/* pciaddress 0000:00:0a:0 (domain:bus:slot:function) */
+
+static int str2PCIAddress(const char *str, struct PCIAddress *pciAddr)
+{
+    char *domain, *bus, *slot, *function;
+
+    if (!pciAddr)
+        return -1;
+    if (!str)
+        return -1;
+
+    domain = (char *)str;
+
+    if (virStrToLong_ui(domain, &bus, 0, &pciAddr->domain) != 0)
+        return -1;
+
+    bus++;
+    if (virStrToLong_ui(bus, &slot, 0, &pciAddr->bus) != 0)
+        return -1;
+
+    slot++;
+    if (virStrToLong_ui(slot, &function, 0, &pciAddr->slot) != 0)
+        return -1;
+
+    function++;
+    if (virStrToLong_ui(function, NULL, 0, &pciAddr->function) != 0)
+        return -1;
+
+    return 0;
+}
+
 static bool
 cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
 {
     virDomainPtr dom = NULL;
     const char *source = NULL, *target = NULL, *driver = NULL,
-                *subdriver = NULL, *type = NULL, *mode = NULL;
+                *subdriver = NULL, *type = NULL, *mode = NULL,
+                *cache = NULL, *serial = NULL, *strpciaddr = NULL;
+    struct PCIAddress pciAddr;
     bool isFile = false, functionReturn = false;
     int ret;
     unsigned int flags;
@@ -10209,6 +10254,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
         goto cleanup;
     }
 
+    ignore_value(vshCommandOptString(cmd, "cache", &cache));
+    ignore_value(vshCommandOptString(cmd, "serial", &serial));
+    ignore_value(vshCommandOptString(cmd, "pciaddress", &strpciaddr));
+
     if (!stype) {
         if (driver && (STREQ(driver, "file") || STREQ(driver, "tap")))
             isFile = true;
@@ -10241,8 +10290,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
         virBufferAsprintf(&buf, " name='%s'", driver);
     if (subdriver)
         virBufferAsprintf(&buf, " type='%s'", subdriver);
+    if (cache)
+        virBufferAsprintf(&buf, " cache='%s'", cache);
 
-    if (driver || subdriver)
+    if (driver || subdriver || cache)
         virBufferAddLit(&buf, "/>\n");
 
     virBufferAsprintf(&buf, "  <source %s='%s'/>\n",
@@ -10252,6 +10303,29 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
     if (mode)
         virBufferAsprintf(&buf, "  <%s/>\n", mode);
 
+    if (serial)
+        virBufferAsprintf(&buf, "  <serial>%s</serial>\n", serial);
+
+    if (vshCommandOptBool(cmd, "shareable"))
+        virBufferAsprintf(&buf, "  <shareable/>\n");
+
+    if (strpciaddr) {
+        if (!STRPREFIX((const char *)target, "vd")) {
+            vshError(ctl, _("PCI address can be set only for vdx disks."));
+            goto cleanup;
+        }
+        if (str2PCIAddress(strpciaddr, &pciAddr) == 0) {
+            virBufferAsprintf(&buf,
+                              "  <address type='pci' domain='0x%04x'"
+                              " bus ='0x%02x' slot='0x%02x' function='0x%0x' />\n",
+                              pciAddr.domain, pciAddr.bus,
+                              pciAddr.slot, pciAddr.function);
+        } else {
+            vshError(ctl, _("Invalid PCI address."));
+            goto cleanup;
+        }
+    }
+
     virBufferAddLit(&buf, "</disk>\n");
 
     if (virBufferError(&buf)) {
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 8b820d2..306f52e 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -876,8 +876,9 @@ For cdrom and floppy devices, this command only replaces the media within
 the single existing device; consider using B<update-device> for this usage.
 
 =item B<attach-disk> I<domain-id> I<source> I<target> optional
-I<--driver driver> I<--subdriver subdriver> I<--type type>
-I<--mode mode> I<--persistent> I<--sourcetype soucetype>
+I<--driver driver> I<--subdriver subdriver> I<--cache cache>
+I<--type type> I<--mode mode> I<--persistent> I<--sourcetype soucetype>
+I<--serial serial> I<--shareable> I<--pciaddress pciaddress>
 
 Attach a new disk device to the domain.
 I<source> and I<target> are paths for the files and devices.
@@ -888,6 +889,11 @@ floppy device; consider using B<update-device> for this usage instead.
 I<mode> can specify the two specific mode I<readonly> or I<shareable>.
 I<persistent> indicates the changes will affect the next boot of the domain.
 I<sourcetype> can indicate the type of source (block|file)
+I<cache> can be one of "default", "none", "writethrough" or "writeback".
+I<serial> is the serial of disk device. I<shareable> indicates the disk device
+is shareable between domains.
+I<pciaddress> is the pci address of disk controller. This option can be set
+only for vdx disks.
 
 =item B<attach-interface> I<domain-id> I<type> I<source> optional
 I<--target target> I<--mac mac> I<--script script> I<--model model>
-- 
1.7.5.1


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]