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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list