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

Re: [libvirt] [RFC v3 PATCH 5/5] PowerPC : Add address-type "spapr-vio"



On Thu, 2011-12-01 at 09:52 +0000, Daniel P. Berrange wrote:
> On Thu, Dec 01, 2011 at 05:38:31PM +1100, Michael Ellerman wrote:
> > But if it's a hard requirement in your view then I'll do it.
> 
> It is the only way to ensure stable device addresses every time the
> guest is started.
> 
> eg, consider what happens if you don't assign addresses
> 
>    qemu -drive foo  -drive bar
> 
> QEMU assigns  foo reg=1,  bar reg=2. Now delete hotunplug a device
> 
>   (qemu) drive_del foo
> 
> Now migrate QEMU to a new host, which means it'll be started with
> 
>    qemu -drive bar
> 
> So QEMU will assign bar reg=1, whereas the guest OS currently
> thinks it has reg=2.
> 
> libvirt *must* define addresses for every single device it passes to
> QEMU at all times. Even if we ignore hotplug, and just consider that
> we cold unpluged 'foo', we still want 'bar' to have the same address,
> in case the guest OS had configured something based on address, or
> worse yet done an OS license/activation check based on hardware config.

OK. Currently I don't think either of those examples actually apply to
us, but I can see that it is liable to bite us in the future if we don't
assign addresses.

Below is a first cut at a patch to do this. It seems to work but it has
a few problems.

Firstly there are calls in qemuDomainUpdateDeviceConfig()
to qemuDomainAssignPCIAddresses(), I'm not sure if I need to also add
calls to qemuAssignSpaprVIOAddresses() in there. Can you explain to me
when qemuDomainUpdateDeviceConfig() is called and what it is doing?

Secondly, and this is a real problem, if the user specifies a serial
with no address type, we don't see it, because it doesn't have a
spapr-vio address, and so we don't take it into account in the address
allocation. But then because it ends becoming a spapr-vty in
qemuBuildChrDeviceStr() (which Prerna added), it clashes with the other
serial we created.

Example config is:

 <serial type="pty"/>
 <serial type="pty">
   <address type="spapr-vio"/>
 </serial>

So we need some way of knowing that the first serial will end up on the
spapr-vio bus, so we can take it into account in the address allocation.
I haven't thought of a good way to fix that yet. We could just say that
it's unsupported for pseries, ie. you MUST specify the address type, but
that would be a pity.

cheers


diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index eceae35..dfd92ad 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -684,6 +684,63 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virBitmapPtr qemuCaps)
     return -1;
 }
 
+static int qemuSpaprVIOFindByReg(virDomainDefPtr def ATTRIBUTE_UNUSED,
+                                 virDomainDeviceInfoPtr info, void *opaque)
+{
+    virDomainDeviceInfoPtr target = (virDomainDeviceInfoPtr)opaque;
+
+    if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)
+        return 0;
+
+    /* Match a dev that has a reg, is not us, and has a matching reg */
+    if (info->addr.spaprvio.has_reg && info != target &&
+        info->addr.spaprvio.reg == target->addr.spaprvio.reg)
+        /* Has to be < 0 so virDomainDeviceInfoIterate() will exit */
+        return -1;
+
+    return 0;
+}
+
+static void qemuAssignSpaprVIOAddress(virDomainDefPtr def,
+                                      virDomainDeviceInfoPtr info,
+                                      unsigned long long default_reg)
+{
+    int rc;
+
+    if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)
+        return;
+
+    if (info->addr.spaprvio.has_reg)
+        return;
+
+    info->addr.spaprvio.has_reg = true;
+    info->addr.spaprvio.reg = default_reg;
+
+    rc = virDomainDeviceInfoIterate(def, qemuSpaprVIOFindByReg, info);
+    while (rc != 0) {
+        /* We hit another device, move along by 4K and search again */
+        info->addr.spaprvio.reg += 0x1000;
+        rc = virDomainDeviceInfoIterate(def, qemuSpaprVIOFindByReg, info);
+    }
+}
+
+int qemuAssignSpaprVIOAddresses(virDomainDefPtr def)
+{
+    int i;
+
+    for (i = 0 ; i < def->nnets; i++)
+        qemuAssignSpaprVIOAddress(def, &def->nets[i]->info, 0x1000ul);
+
+    for (i = 0 ; i < def->ncontrollers; i++)
+        qemuAssignSpaprVIOAddress(def, &def->controllers[i]->info, 0x2000ul);
+
+    for (i = 0 ; i < def->nserials; i++)
+        qemuAssignSpaprVIOAddress(def, &def->serials[i]->info, 0x30000000ul);
+
+    /* No other devices are currently supported on spapr-vio */
+
+    return 0;
+}
 
 #define QEMU_PCI_ADDRESS_LAST_SLOT 31
 #define QEMU_PCI_ADDRESS_LAST_FUNCTION 8
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 8b51ef3..f748c2b 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -203,6 +203,8 @@ int qemuAssignDeviceHostdevAlias(virDomainDefPtr def, virDomainHostdevDefPtr hos
 int qemuAssignDeviceControllerAlias(virDomainControllerDefPtr controller);
 int qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, virDomainRedirdevDefPtr redirdev, int idx);
 
+int qemuAssignSpaprVIOAddresses(virDomainDefPtr def);
+
 int
 qemuParseKeywords(const char *str,
                   char ***retkeywords,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 105bdde..6c47516 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1317,6 +1317,8 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml,
     if (qemuDomainAssignPCIAddresses(def) < 0)
         goto cleanup;
 
+    qemuAssignSpaprVIOAddresses(def);
+
     if (!(vm = virDomainAssignDef(driver->caps,
                                   &driver->domains,
                                   def, false)))
@@ -4903,6 +4905,8 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) {
     if (qemuDomainAssignPCIAddresses(def) < 0)
         goto cleanup;
 
+    qemuAssignSpaprVIOAddresses(def);
+
     if (!(vm = virDomainAssignDef(driver->caps,
                                   &driver->domains,
                                   def, false))) {
@@ -10748,6 +10752,8 @@ static virDomainPtr qemuDomainAttach(virConnectPtr conn,
     if (qemuDomainAssignPCIAddresses(def) < 0)
         goto cleanup;
 
+    qemuAssignSpaprVIOAddresses(def);
+
     if (!(vm = virDomainAssignDef(driver->caps,
                                   &driver->domains,
                                   def, false)))
-- 
1.7.7.3




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