Re: [PATCH v3 1/3] Add NVRAM device

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

 



On 2013年04月11日 17:36, Daniel P. Berrange wrote:
On Wed, Mar 27, 2013 at 01:07:54PM +0800, Li Zhang wrote:
From: Li Zhang <zhlcindy@xxxxxxxxxxxxxxxxxx>

For pSeries guest in QEMU, NVRAM is one kind of spapr-vio device.
Users are allowed to specify spapr-vio devices'address.
But NVRAM is not supported in libvirt. So this patch is to
add NVRAM device to allow users to specify its address.

In QEMU, NVRAM device's address is specified by
  "-global spapr-nvram.reg=xxxxx".

In libvirt, XML file is defined as the following:

   <nvram>
     <address type='spapr-vio' reg='0x3000'/>
   </nvram>

Signed-off-by: Li Zhang <zhlcindy@xxxxxxxxxxxxxxxxxx>
---
  v3 -> v2:
   * Add NVRAM in domaincommon.rng and formatdomain.xml.in suggested by Daniel P.Berrange
   * Add NVRAM test cases suggested by Daniel P.Berrange
   * Remove nvram allocation when it is not specified suggested by Daniel P.Berrange
   * Add several error reports suggested by Daniel P.Berrange

  v2 -> v1:
   * Add NVRAM parameters parsing in qemuParseCommandLine

  src/conf/domain_conf.c  |   76 +++++++++++++++++++++++++++++++++++++++++++++++
  src/conf/domain_conf.h  |   10 +++++++

  src/qemu/qemu_command.c |   67 +++++++++++++++++++++++++++++++++++++++++
  src/qemu/qemu_command.h |    2 ++
  4 files changed, 155 insertions(+)
The QEMU pieces should be separate from the parser pieces. Basically instead
of the 3 patches you have here you should have 2 patches with the following
files in each one:

  1.  src/conf/domain_conf.*, docs/schemas/* and
      docs/formatdomain.html.in

  2. src/qemu/* and tests/qemuxml2argv*
OK, got it.
@@ -13854,6 +13911,21 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
  }
static int
+virDomainNVRAMDefFormat(virBufferPtr buf,
+                             virDomainNVRAMDefPtr def,
+                             unsigned int flags)
+{
+    virBufferAsprintf(buf, "    <nvram>\n");
+    if (virDomainDeviceInfoIsSet(&def->info, flags))
+        if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+            return -1;
+
+     virBufferAddLit(buf, "    </nvram>\n");
+
+     return 0;
There's inconsistent indentation here

I will correct it.
  qemuBuildUSBInputDevStr(virDomainInputDefPtr dev,
@@ -7492,6 +7527,21 @@ qemuBuildCommandLine(virConnectPtr conn,
              goto error;
      }
+ if (def->nvram) {
+        if (def->os.arch == VIR_ARCH_PPC64 &&
+            STREQ(def->os.machine, "pseries")) {
+            char *optstr;
+            virCommandAddArg(cmd, "-global");
+            optstr = qemuBuildNVRAMDevStr(def->nvram);
+            if (!optstr)
+                goto error;
+            if (optstr)
+                virCommandAddArg(cmd, optstr);
+            VIR_FREE(optstr);
+        } else
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                          _("NVRAM device is not supported"));

Missing a 'goto error' here

Ok, I will add it.

@@ -9584,6 +9634,23 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
                  goto error;
              }
+ } else if (STREQ(arg, "-global") &&
+                   STRPREFIX(progargv[i + 1], "spapr-nvram.reg=")) {
+
+            WANT_VALUE();
+
+            if (VIR_ALLOC(def->nvram) < 0)
+                goto no_memory;
+
+            val += strlen("spapr-nvram.reg=");
+            if (virStrToLong_ull(val, NULL, 16,
+                                 &def->nvram->info.addr.spaprvio.reg) < 0) {
Don't you need to set the address type too.

Ah, yes. I will add it.

+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("invalid value for spapr-nvram.reg :"
+                                 "'%s'"), val);
+                goto error;
+            }
+
          } else if (STREQ(arg, "-S")) {
              /* ignore, always added by libvirt */
          } else {


Daniel

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]