3:10 a.m.
As discussed earlier [1], we should allow users to set device
aliases at the define time. While the discussed approach calls
for generating missing aliases too, I'm saving that for another
patch set. There are couple of reasons for that:
a) I don't think it's really necessary (if users are interested
in a device they can just set the alias).
b) This patch set is already big enough as is.
c) Generating aliases is going to have its own problems.
Therefore, for now I'm only proposing parsing user supplied
aliases. The idea is that it's not enabled by default for all
drivers. Any driver that want to/can provide this feature has to
set VIR_DOMAIN_DEF_FEATURE_USER_ALIAS. Note that we have some
drivers that don't have notion of device aliases. But the code is
generic enough so that it should be easy to use in the drivers
that do know what aliases are.
1: https://www.redhat.com/archives/libvir-list/2017-October/msg00201.html
Michal Privoznik (14):
conf: Fix virDomainDeviceGetInfo const correctness
virDomainObjGetOneDefState: Fix error message
qemuAssignDeviceAliases: Use qemuAssignDeviceRNGAlias for assigning
RNG aliases
qemu: Move device alias assignment to separate functions
qemu: Be tolerant to preexisting aliases
conf: Pass xmlopt down to virDomainDeviceInfoParseXML
conf: Parse user supplied aliases
conf: Validate user supplied aliases
virDomainDeviceInfoCheckABIStability: Check for alias too
qemuxml2argvdata: Drop device aliases
qemuhotplugtest: Load active XML
conf: Format alias even for inactive XMLs
docs: Document user aliases
qemu: Parse alias from inactive XMLs
docs/formatdomain.html.in | 23 ++
src/conf/domain_conf.c | 353 ++++++++++++++++-----
src/conf/domain_conf.h | 8 +-
src/libvirt_private.syms | 1 +
src/qemu/qemu_alias.c | 139 +++++++-
src/qemu/qemu_domain.c | 3 +-
src/qemu/qemu_driver.c | 3 +
src/qemu/qemu_hotplug.c | 6 +-
tests/qemuhotplugtest.c | 3 +-
.../qemuxml2argv-disk-cdrom-network-ftp.xml | 1 -
.../qemuxml2argv-disk-cdrom-network-ftps.xml | 1 -
.../qemuxml2argv-disk-cdrom-network-http.xml | 1 -
.../qemuxml2argv-disk-cdrom-network-https.xml | 1 -
.../qemuxml2argv-disk-cdrom-network-tftp.xml | 1 -
.../qemuxml2argv-usb-redir-filter.xml | 1 -
15 files changed, 444 insertions(+), 101 deletions(-)
--
2.13.6
3:10 a.m.
New subject: [libvirt] [PATCH v1 01/14] conf: Fix virDomainDeviceGetInfo const correctness
This function is not changing passed domain definition.
Therefore, mark the argument as 'const'.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 2 +-
src/conf/domain_conf.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 25d48f977..31684df54 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3501,7 +3501,7 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info,
}
virDomainDeviceInfoPtr
-virDomainDeviceGetInfo(virDomainDeviceDefPtr device)
+virDomainDeviceGetInfo(const virDomainDeviceDef *device)
{
switch ((virDomainDeviceType) device->type) {
case VIR_DOMAIN_DEVICE_DISK:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a42efcfa6..1e007346f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2756,7 +2756,7 @@ virDomainDeviceDefPtr virDomainDeviceDefCopy(virDomainDeviceDefPtr
src,
virDomainXMLOptionPtr xmlopt);
int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info,
int type);
-virDomainDeviceInfoPtr virDomainDeviceGetInfo(virDomainDeviceDefPtr device);
+virDomainDeviceInfoPtr virDomainDeviceGetInfo(const virDomainDeviceDef *device);
void virDomainTPMDefFree(virDomainTPMDefPtr def);
typedef int (*virDomainDeviceInfoCallback)(virDomainDefPtr def,
--
2.13.6
4:55 a.m.
New subject: [libvirt] [PATCH v1 01/14] conf: Fix virDomainDeviceGetInfo const correctness
On Thu, Oct 19, 2017 at 10:10:56AM +0200, Michal Privoznik wrote:
This function is not changing passed domain definition.
Therefore, mark the argument as 'const'.
Is there any other reason for this then just your new code using const?
I don't see any pointer that's passed through your later code that would
be const nowadays, so why would we play the const-correctness game here
when it doesn't make much sense in C when the pointer is to a struct
since the const doesn't propagate?
If s/const virDomainDeviceDef/virDomainDeviceDef/ on your later patch
makes this one unnecessary, then NACK to this. If there is a reason,
however, feel free to keep this in.
10:34 a.m.
New subject: [libvirt] [PATCH v1 01/14] conf: Fix virDomainDeviceGetInfo const correctness
On 10/19/2017 11:55 AM, Martin Kletzander wrote:
On Thu, Oct 19, 2017 at 10:10:56AM +0200, Michal Privoznik wrote:
> This function is not changing passed domain definition.
> Therefore, mark the argument as 'const'.
>
Is there any other reason for this then just your new code using const?
I don't see any pointer that's passed through your later code that would
be const nowadays, so why would we play the const-correctness game here
when it doesn't make much sense in C when the pointer is to a struct
since the const doesn't propagate?
If s/const virDomainDeviceDef/virDomainDeviceDef/ on your later patch
makes this one unnecessary, then NACK to this. If there is a reason,
however, feel free to keep this in.
Well, later in 08/14 I am indeed calling this from function which takes
const virDomainDeviceDef * and I can change it to virDomainDeviceDefPtr.
And while I think this patch still makes things better (solves const
correctness), I don't want it to be show stopper. Thus I'm dropping it.
Michal
3:10 a.m.
New subject: [libvirt] [PATCH v1 02/14] virDomainObjGetOneDefState: Fix error message
It looks like the error message was copied from virsh, because
that's where we have @ctl. Nevertheless, it's @flags which is
invalid, not @ctl.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 31684df54..b3feddc39 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3418,12 +3418,13 @@ virDomainObjGetOneDefState(virDomainObjPtr vm,
unsigned int flags,
bool *live)
{
- if (flags & VIR_DOMAIN_AFFECT_LIVE && flags &
VIR_DOMAIN_AFFECT_CONFIG) {
- virReportInvalidArg(ctl, "%s",
- _("Flags 'VIR_DOMAIN_AFFECT_LIVE' and
"
- "'VIR_DOMAIN_AFFECT_CONFIG' are mutually
"
- "exclusive"));
- return NULL;
+ if (flags & VIR_DOMAIN_AFFECT_LIVE &&
+ flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ virReportInvalidArg(flags, "%s",
+ _("Flags 'VIR_DOMAIN_AFFECT_LIVE' and "
+ "'VIR_DOMAIN_AFFECT_CONFIG' are mutually
"
+ "exclusive"));
+ return NULL;
}
if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
--
2.13.6
3:10 a.m.
New subject: [libvirt] [PATCH v1 03/14] qemuAssignDeviceAliases: Use qemuAssignDeviceRNGAlias for assigning RNG aliases
We have a special function for assigning aliases to RNG devices.
Use that instead of plain virAsprintf().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_alias.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 72df1083f..eca1dc64c 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -501,7 +501,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
return -1;
}
for (i = 0; i < def->nrngs; i++) {
- if (virAsprintf(&def->rngs[i]->info.alias, "rng%zu", i) <
0)
+ if (qemuAssignDeviceRNGAlias(def, def->rngs[i]) < 0)
return -1;
}
if (def->tpm) {
--
2.13.6
3:10 a.m.
New subject: [libvirt] [PATCH v1 04/14] qemu: Move device alias assignment to separate functions
Let's move all the virAsprintf()-s into separate functions for
better structure of the code. Later, when somebody wants to
generate a device alias, all they need is to expose the function.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_alias.c | 80 +++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 72 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index eca1dc64c..33bd4c10d 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -286,6 +286,70 @@ qemuAssignDeviceNetAlias(virDomainDefPtr def,
}
+static int
+qemuAssignDeviceFSAlias(virDomainFSDefPtr fss,
+ int idx)
+{
+ return virAsprintf(&fss->info.alias, "fs%d", idx);
+}
+
+
+static int
+qemuAssignDeviceSoundAlias(virDomainSoundDefPtr sound,
+ int idx)
+{
+ return virAsprintf(&sound->info.alias, "sound%d", idx);
+}
+
+
+static int
+qemuAssignDeviceVideoAlias(virDomainVideoDefPtr video,
+ int idx)
+{
+ return virAsprintf(&video->info.alias, "video%d", idx);
+}
+
+
+static int
+qemuAssignDeviceInputAlias(virDomainInputDefPtr input,
+ int idx)
+{
+ return virAsprintf(&input->info.alias, "input%d", idx);
+}
+
+
+static int
+qemuAssignDeviceHubAlias(virDomainHubDefPtr hub,
+ int idx)
+{
+ return virAsprintf(&hub->info.alias, "hub%d", idx);
+}
+
+
+static int
+qemuAssignDeviceSmartcardAlias(virDomainSmartcardDefPtr smartcard,
+ int idx)
+{
+ return virAsprintf(&smartcard->info.alias, "smartcard%d", idx);
+}
+
+
+static int
+qemuAssingDeviceMemballoonAlias(virDomainMemballoonDefPtr memballoon,
+ int idx)
+{
+ return virAsprintf(&memballoon->info.alias, "balloon%d", idx);
+}
+
+
+static int
+qemuAssignDeviceTPMAlias(virDomainTPMDefPtr tpm,
+ int idx)
+{
+ return virAsprintf(&tpm->info.alias, "tpm%d", idx);
+}
+
+
int
qemuAssignDeviceRedirdevAlias(virDomainDefPtr def,
virDomainRedirdevDefPtr redirdev,
@@ -431,11 +495,11 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr
qemuCaps)
}
for (i = 0; i < def->nfss; i++) {
- if (virAsprintf(&def->fss[i]->info.alias, "fs%zu", i) <
0)
+ if (qemuAssignDeviceFSAlias(def->fss[i], i) < 0)
return -1;
}
for (i = 0; i < def->nsounds; i++) {
- if (virAsprintf(&def->sounds[i]->info.alias, "sound%zu", i)
< 0)
+ if (qemuAssignDeviceSoundAlias(def->sounds[i], i) < 0)
return -1;
}
for (i = 0; i < def->nhostdevs; i++) {
@@ -453,7 +517,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
return -1;
}
for (i = 0; i < def->nvideos; i++) {
- if (virAsprintf(&def->videos[i]->info.alias, "video%zu", i)
< 0)
+ if (qemuAssignDeviceVideoAlias(def->videos[i], i) < 0)
return -1;
}
for (i = 0; i < def->ncontrollers; i++) {
@@ -461,7 +525,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
return -1;
}
for (i = 0; i < def->ninputs; i++) {
- if (virAsprintf(&def->inputs[i]->info.alias, "input%zu", i)
< 0)
+ if (qemuAssignDeviceInputAlias(def->inputs[i], i) < 0)
return -1;
}
for (i = 0; i < def->nparallels; i++) {
@@ -481,7 +545,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
return -1;
}
for (i = 0; i < def->nhubs; i++) {
- if (virAsprintf(&def->hubs[i]->info.alias, "hub%zu", i) <
0)
+ if (qemuAssignDeviceHubAlias(def->hubs[i], i) < 0)
return -1;
}
for (i = 0; i < def->nshmems; i++) {
@@ -489,7 +553,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
return -1;
}
for (i = 0; i < def->nsmartcards; i++) {
- if (virAsprintf(&def->smartcards[i]->info.alias,
"smartcard%zu", i) < 0)
+ if (qemuAssignDeviceSmartcardAlias(def->smartcards[i], i) < 0)
return -1;
}
if (def->watchdog) {
@@ -497,7 +561,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
return -1;
}
if (def->memballoon) {
- if (virAsprintf(&def->memballoon->info.alias, "balloon%d", 0)
< 0)
+ if (qemuAssingDeviceMemballoonAlias(def->memballoon, 0) < 0)
return -1;
}
for (i = 0; i < def->nrngs; i++) {
@@ -505,7 +569,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
return -1;
}
if (def->tpm) {
- if (virAsprintf(&def->tpm->info.alias, "tpm%d", 0) < 0)
+ if (qemuAssignDeviceTPMAlias(def->tpm, 0) < 0)
return -1;
}
for (i = 0; i < def->nmems; i++) {
--
2.13.6
4:55 a.m.
New subject: [libvirt] [PATCH v1 04/14] qemu: Move device alias assignment to separate functions
On Thu, Oct 19, 2017 at 10:10:59AM +0200, Michal Privoznik wrote:
Let's move all the virAsprintf()-s into separate functions for
better structure of the code. Later, when somebody wants to
generate a device alias, all they need is to expose the function.
I'm just waiting for these functions to totally fail when we introduce a hotplug
of such device. There could be a universal wrapper for these, but that's not
the scope of this patchset.
ACK
3:11 a.m.
New subject: [libvirt] [PATCH v1 05/14] qemu: Be tolerant to preexisting aliases
In the future, some aliases might be already parsed therefore we
should avoid overwriting them.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_alias.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++--
src/qemu/qemu_hotplug.c | 6 ++----
2 files changed, 57 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 33bd4c10d..7a3983afa 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -93,6 +93,9 @@ qemuAssignDeviceChrAlias(virDomainDefPtr def,
{
const char *prefix = NULL;
+ if (chr->info.alias)
+ return 0;
+
switch ((virDomainChrDeviceType) chr->deviceType) {
case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL:
prefix = "parallel";
@@ -128,6 +131,9 @@ qemuAssignDeviceControllerAlias(virDomainDefPtr domainDef,
{
const char *prefix = virDomainControllerTypeToString(controller->type);
+ if (controller->info.alias)
+ return 0;
+
if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
if (!virQEMUCapsHasPCIMultiBus(qemuCaps, domainDef)) {
/* qemus that don't support multiple PCI buses have
@@ -182,6 +188,9 @@ qemuAssignDeviceDiskAlias(virDomainDefPtr def,
const char *prefix = virDomainDiskBusTypeToString(disk->bus);
int controllerModel = -1;
+ if (disk->info.alias)
+ return 0;
+
if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
controllerModel =
@@ -223,6 +232,9 @@ qemuAssignDeviceHostdevAlias(virDomainDefPtr def,
char **alias,
int idx)
{
+ if (*alias)
+ return 0;
+
if (idx == -1) {
size_t i;
@@ -259,6 +271,9 @@ qemuAssignDeviceNetAlias(virDomainDefPtr def,
int idx)
{
+ if (net->info.alias)
+ return 0;
+
/* <interface type='hostdev'> uses "hostdevN" as the alias
* We must use "-1" as the index because the caller doesn't know
* that we're now looking for a unique hostdevN rather than netN
@@ -290,6 +305,9 @@ static int
qemuAssignDeviceFSAlias(virDomainFSDefPtr fss,
int idx)
{
+ if (fss->info.alias)
+ return 0;
+
return virAsprintf(&fss->info.alias, "fs%d", idx);
}
@@ -298,6 +316,9 @@ static int
qemuAssignDeviceSoundAlias(virDomainSoundDefPtr sound,
int idx)
{
+ if (sound->info.alias)
+ return 0;
+
return virAsprintf(&sound->info.alias, "sound%d", idx);
}
@@ -306,6 +327,9 @@ static int
qemuAssignDeviceVideoAlias(virDomainVideoDefPtr video,
int idx)
{
+ if (video->info.alias)
+ return 0;
+
return virAsprintf(&video->info.alias, "video%d", idx);
}
@@ -314,6 +338,9 @@ static int
qemuAssignDeviceInputAlias(virDomainInputDefPtr input,
int idx)
{
+ if (input->info.alias)
+ return 0;
+
return virAsprintf(&input->info.alias, "input%d", idx);
}
@@ -322,6 +349,9 @@ static int
qemuAssignDeviceHubAlias(virDomainHubDefPtr hub,
int idx)
{
+ if (hub->info.alias)
+ return 0;
+
return virAsprintf(&hub->info.alias, "hub%d", idx);
}
@@ -330,6 +360,9 @@ static int
qemuAssignDeviceSmartcardAlias(virDomainSmartcardDefPtr smartcard,
int idx)
{
+ if (smartcard->info.alias)
+ return 0;
+
return virAsprintf(&smartcard->info.alias, "smartcard%d", idx);
}
@@ -338,6 +371,9 @@ static int
qemuAssingDeviceMemballoonAlias(virDomainMemballoonDefPtr memballoon,
int idx)
{
+ if (memballoon->info.alias)
+ return 0;
+
return virAsprintf(&memballoon->info.alias, "balloon%d", idx);
}
@@ -346,6 +382,9 @@ static int
qemuAssignDeviceTPMAlias(virDomainTPMDefPtr tpm,
int idx)
{
+ if (tpm->info.alias)
+ return 0;
+
return virAsprintf(&tpm->info.alias, "tpm%d", idx);
}
@@ -355,6 +394,9 @@ qemuAssignDeviceRedirdevAlias(virDomainDefPtr def,
virDomainRedirdevDefPtr redirdev,
int idx)
{
+ if (redirdev->info.alias)
+ return 0;
+
if (idx == -1) {
size_t i;
idx = 0;
@@ -384,6 +426,9 @@ qemuAssignDeviceRNGAlias(virDomainDefPtr def,
int maxidx = 0;
int idx;
+ if (rng->info.alias)
+ return 0;
+
for (i = 0; i < def->nrngs; i++) {
if ((idx = qemuDomainDeviceAliasIndex(&def->rngs[i]->info,
"rng")) >= maxidx)
maxidx = idx + 1;
@@ -418,6 +463,9 @@ qemuAssignDeviceMemoryAlias(virDomainDefPtr def,
int idx;
const char *prefix;
+ if (mem->info.alias)
+ return 0;
+
if (mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM)
prefix = "dimm";
else
@@ -444,6 +492,9 @@ qemuAssignDeviceShmemAlias(virDomainDefPtr def,
virDomainShmemDefPtr shmem,
int idx)
{
+ if (shmem->info.alias)
+ return 0;
+
if (idx == -1) {
size_t i;
idx = 0;
@@ -474,6 +525,9 @@ qemuAssignDeviceWatchdogAlias(virDomainWatchdogDefPtr watchdog)
{
/* Currently, there's just one watchdog per domain */
+ if (watchdog->info.alias)
+ return 0;
+
if (VIR_STRDUP(watchdog->info.alias, "watchdog0") < 0)
return -1;
return 0;
@@ -508,8 +562,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
* linked to a NetDef, they will share an info and the alias
* will already be set, so don't try to set it again.
*/
- if (!def->hostdevs[i]->info->alias &&
- qemuAssignDeviceHostdevAlias(def,
&def->hostdevs[i]->info->alias, -1) < 0)
+ if (qemuAssignDeviceHostdevAlias(def,
&def->hostdevs[i]->info->alias, -1) < 0)
return -1;
}
for (i = 0; i < def->nredirdevs; i++) {
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0288986d8..d556c9be0 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4980,10 +4980,8 @@ qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver,
{
int ret = -1;
- if (!detach->info->alias) {
- if (qemuAssignDeviceHostdevAlias(vm->def, &detach->info->alias, -1)
< 0)
- return -1;
- }
+ if (qemuAssignDeviceHostdevAlias(vm->def, &detach->info->alias, -1) <
0)
+ return -1;
switch (detach->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
--
2.13.6
3:11 a.m.
New subject: [libvirt] [PATCH v1 06/14] conf: Pass xmlopt down to virDomainDeviceInfoParseXML
This function is going to make decisions based on the features
set per each driver. For that we need the virDomainXMLOption
object.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 165 ++++++++++++++++++++++++++++++-------------------
1 file changed, 102 insertions(+), 63 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b3feddc39..f4de4e288 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6402,7 +6402,8 @@ virDomainDeviceAddressParseXML(xmlNodePtr address,
* @param node XML nodeset to parse for device address definition
*/
static int
-virDomainDeviceInfoParseXML(xmlNodePtr node,
+virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED,
+ xmlNodePtr node,
virHashTablePtr bootHash,
virDomainDeviceInfoPtr info,
unsigned int flags)
@@ -9177,7 +9178,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
}
def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
} else {
- if (virDomainDeviceInfoParseXML(node, bootHash, &def->info,
+ if (virDomainDeviceInfoParseXML(xmlopt, node, bootHash, &def->info,
flags | VIR_DOMAIN_DEF_PARSE_ALLOW_BOOT) < 0)
goto error;
}
@@ -9514,7 +9515,8 @@ virDomainControllerModelTypeToString(virDomainControllerDefPtr def,
* @param node XML nodeset to parse for controller definition
*/
static virDomainControllerDefPtr
-virDomainControllerDefParseXML(xmlNodePtr node,
+virDomainControllerDefParseXML(virDomainXMLOptionPtr xmlopt,
+ xmlNodePtr node,
xmlXPathContextPtr ctxt,
unsigned int flags)
{
@@ -9663,7 +9665,8 @@ virDomainControllerDefParseXML(xmlNodePtr node,
if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
def->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) {
VIR_DEBUG("Ignoring device address for none model usb controller");
- } else if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
{
+ } else if (virDomainDeviceInfoParseXML(xmlopt, node, NULL,
+ &def->info, flags) < 0) {
goto error;
}
@@ -9890,7 +9893,8 @@ virDomainNetGenerateMAC(virDomainXMLOptionPtr xmlopt,
* @param node XML nodeset to parse for disk definition
*/
static virDomainFSDefPtr
-virDomainFSDefParseXML(xmlNodePtr node,
+virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
+ xmlNodePtr node,
xmlXPathContextPtr ctxt,
unsigned int flags)
{
@@ -10047,7 +10051,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
def->dst = target;
target = NULL;
- if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
goto error;
cleanup:
@@ -10568,7 +10572,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
}
def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
} else {
- if (virDomainDeviceInfoParseXML(node, bootHash, &def->info,
+ if (virDomainDeviceInfoParseXML(xmlopt, node, bootHash, &def->info,
flags | VIR_DOMAIN_DEF_PARSE_ALLOW_BOOT
| VIR_DOMAIN_DEF_PARSE_ALLOW_ROM) < 0)
goto error;
@@ -11835,7 +11839,8 @@ virDomainChrDefParseXML(virDomainXMLOptionPtr xmlopt,
if (def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD) {
VIR_DEBUG("Ignoring device address for gustfwd channel");
- } else if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
{
+ } else if (virDomainDeviceInfoParseXML(xmlopt, node, NULL,
+ &def->info, flags) < 0) {
goto error;
}
@@ -11972,7 +11977,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt,
goto error;
}
- if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
goto error;
if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID) {
@@ -12005,7 +12010,8 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt,
*
*/
static virDomainTPMDefPtr
-virDomainTPMDefParseXML(xmlNodePtr node,
+virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
+ xmlNodePtr node,
xmlXPathContextPtr ctxt,
unsigned int flags)
{
@@ -12074,7 +12080,7 @@ virDomainTPMDefParseXML(xmlNodePtr node,
goto error;
}
- if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
goto error;
cleanup:
@@ -12093,7 +12099,8 @@ virDomainTPMDefParseXML(xmlNodePtr node,
}
static virDomainPanicDefPtr
-virDomainPanicDefParseXML(xmlNodePtr node,
+virDomainPanicDefParseXML(virDomainXMLOptionPtr xmlopt,
+ xmlNodePtr node,
unsigned int flags)
{
virDomainPanicDefPtr panic;
@@ -12102,7 +12109,8 @@ virDomainPanicDefParseXML(xmlNodePtr node,
if (VIR_ALLOC(panic) < 0)
return NULL;
- if (virDomainDeviceInfoParseXML(node, NULL, &panic->info, flags) < 0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, NULL,
+ &panic->info, flags) < 0)
goto error;
model = virXMLPropString(node, "model");
@@ -12125,7 +12133,8 @@ virDomainPanicDefParseXML(xmlNodePtr node,
/* Parse the XML definition for an input device */
static virDomainInputDefPtr
-virDomainInputDefParseXML(const virDomainDef *dom,
+virDomainInputDefParseXML(virDomainXMLOptionPtr xmlopt,
+ const virDomainDef *dom,
xmlNodePtr node,
xmlXPathContextPtr ctxt,
unsigned int flags)
@@ -12236,7 +12245,7 @@ virDomainInputDefParseXML(const virDomainDef *dom,
}
}
- if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
goto error;
if (def->bus == VIR_DOMAIN_INPUT_BUS_USB &&
@@ -12276,7 +12285,9 @@ virDomainInputDefParseXML(const virDomainDef *dom,
/* Parse the XML definition for a hub device */
static virDomainHubDefPtr
-virDomainHubDefParseXML(xmlNodePtr node, unsigned int flags)
+virDomainHubDefParseXML(virDomainXMLOptionPtr xmlopt,
+ xmlNodePtr node,
+ unsigned int flags)
{
virDomainHubDefPtr def;
char *type = NULL;
@@ -12298,7 +12309,7 @@ virDomainHubDefParseXML(xmlNodePtr node, unsigned int flags)
goto error;
}
- if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
goto error;
cleanup:
@@ -13375,7 +13386,8 @@ virDomainSoundCodecDefParseXML(xmlNodePtr node)
static virDomainSoundDefPtr
-virDomainSoundDefParseXML(xmlNodePtr node,
+virDomainSoundDefParseXML(virDomainXMLOptionPtr xmlopt,
+ xmlNodePtr node,
xmlXPathContextPtr ctxt,
unsigned int flags)
{
@@ -13427,7 +13439,7 @@ virDomainSoundDefParseXML(xmlNodePtr node,
}
}
- if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
goto error;
cleanup:
@@ -13444,7 +13456,8 @@ virDomainSoundDefParseXML(xmlNodePtr node,
static virDomainWatchdogDefPtr
-virDomainWatchdogDefParseXML(xmlNodePtr node,
+virDomainWatchdogDefParseXML(virDomainXMLOptionPtr xmlopt,
+ xmlNodePtr node,
unsigned int flags)
{
@@ -13480,7 +13493,7 @@ virDomainWatchdogDefParseXML(xmlNodePtr node,
}
}
- if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
goto error;
cleanup:
@@ -13592,7 +13605,7 @@ virDomainRNGDefParseXML(virDomainXMLOptionPtr xmlopt,
break;
}
- if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
goto error;
if (virDomainVirtioOptionsParseXML(virXPathNode("./driver", ctxt),
@@ -13615,7 +13628,8 @@ virDomainRNGDefParseXML(virDomainXMLOptionPtr xmlopt,
static virDomainMemballoonDefPtr
-virDomainMemballoonDefParseXML(xmlNodePtr node,
+virDomainMemballoonDefParseXML(virDomainXMLOptionPtr xmlopt,
+ xmlNodePtr node,
xmlXPathContextPtr ctxt,
unsigned int flags)
{
@@ -13661,7 +13675,8 @@ virDomainMemballoonDefParseXML(xmlNodePtr node,
if (def->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE)
VIR_DEBUG("Ignoring device address for none model Memballoon");
- else if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
+ else if (virDomainDeviceInfoParseXML(xmlopt, node, NULL,
+ &def->info, flags) < 0)
goto error;
if (virDomainVirtioOptionsParseXML(virXPathNode("./driver", ctxt),
@@ -13682,7 +13697,8 @@ virDomainMemballoonDefParseXML(xmlNodePtr node,
}
static virDomainNVRAMDefPtr
-virDomainNVRAMDefParseXML(xmlNodePtr node,
+virDomainNVRAMDefParseXML(virDomainXMLOptionPtr xmlopt,
+ xmlNodePtr node,
unsigned int flags)
{
virDomainNVRAMDefPtr def;
@@ -13690,7 +13706,7 @@ virDomainNVRAMDefParseXML(xmlNodePtr node,
if (VIR_ALLOC(def) < 0)
return NULL;
- if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
goto error;
return def;
@@ -13701,7 +13717,8 @@ virDomainNVRAMDefParseXML(xmlNodePtr node,
}
static virDomainShmemDefPtr
-virDomainShmemDefParseXML(xmlNodePtr node,
+virDomainShmemDefParseXML(virDomainXMLOptionPtr xmlopt,
+ xmlNodePtr node,
xmlXPathContextPtr ctxt,
unsigned int flags)
{
@@ -13785,7 +13802,7 @@ virDomainShmemDefParseXML(xmlNodePtr node,
goto cleanup;
}
- if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
goto cleanup;
@@ -14206,7 +14223,8 @@ virDomainVideoDriverDefParseXML(xmlNodePtr node)
}
static virDomainVideoDefPtr
-virDomainVideoDefParseXML(xmlNodePtr node,
+virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
+ xmlNodePtr node,
xmlXPathContextPtr ctxt,
const virDomainDef *dom,
unsigned int flags)
@@ -14320,7 +14338,7 @@ virDomainVideoDefParseXML(xmlNodePtr node,
}
}
- if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
goto error;
def->driver = virDomainVideoDriverDefParseXML(node);
@@ -14388,7 +14406,7 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt,
}
if (def->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
- if (virDomainDeviceInfoParseXML(node, bootHash, def->info,
+ if (virDomainDeviceInfoParseXML(xmlopt, node, bootHash, def->info,
flags | VIR_DOMAIN_DEF_PARSE_ALLOW_BOOT
| VIR_DOMAIN_DEF_PARSE_ALLOW_ROM) < 0)
goto error;
@@ -14475,7 +14493,7 @@ virDomainRedirdevDefParseXML(virDomainXMLOptionPtr xmlopt,
if (def->source->type == VIR_DOMAIN_CHR_TYPE_SPICEVMC)
def->source->data.spicevmc = VIR_DOMAIN_CHR_SPICEVMC_USBREDIR;
- if (virDomainDeviceInfoParseXML(node, bootHash, &def->info,
+ if (virDomainDeviceInfoParseXML(xmlopt, node, bootHash, &def->info,
flags | VIR_DOMAIN_DEF_PARSE_ALLOW_BOOT) < 0)
goto error;
@@ -14887,7 +14905,8 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node,
static virDomainMemoryDefPtr
-virDomainMemoryDefParseXML(xmlNodePtr memdevNode,
+virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt,
+ xmlNodePtr memdevNode,
xmlXPathContextPtr ctxt,
unsigned int flags)
{
@@ -14941,7 +14960,8 @@ virDomainMemoryDefParseXML(xmlNodePtr memdevNode,
if (virDomainMemoryTargetDefParseXML(node, ctxt, def) < 0)
goto error;
- if (virDomainDeviceInfoParseXML(memdevNode, NULL, &def->info, flags) < 0)
+ if (virDomainDeviceInfoParseXML(xmlopt, memdevNode,
+ NULL, &def->info, flags) < 0)
goto error;
ctxt->node = save;
@@ -15081,7 +15101,7 @@ virDomainDeviceDefParse(const char *xmlStr,
goto error;
break;
case VIR_DOMAIN_DEVICE_FS:
- if (!(dev->data.fs = virDomainFSDefParseXML(node, ctxt, flags)))
+ if (!(dev->data.fs = virDomainFSDefParseXML(xmlopt, node, ctxt, flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_NET:
@@ -15091,20 +15111,23 @@ virDomainDeviceDefParse(const char *xmlStr,
goto error;
break;
case VIR_DOMAIN_DEVICE_INPUT:
- if (!(dev->data.input = virDomainInputDefParseXML(def, node,
+ if (!(dev->data.input = virDomainInputDefParseXML(xmlopt, def, node,
ctxt, flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_SOUND:
- if (!(dev->data.sound = virDomainSoundDefParseXML(node, ctxt, flags)))
+ if (!(dev->data.sound = virDomainSoundDefParseXML(xmlopt, node,
+ ctxt, flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_WATCHDOG:
- if (!(dev->data.watchdog = virDomainWatchdogDefParseXML(node, flags)))
+ if (!(dev->data.watchdog = virDomainWatchdogDefParseXML(xmlopt,
+ node, flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_VIDEO:
- if (!(dev->data.video = virDomainVideoDefParseXML(node, ctxt, def, flags)))
+ if (!(dev->data.video = virDomainVideoDefParseXML(xmlopt, node,
+ ctxt, def, flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_HOSTDEV:
@@ -15114,8 +15137,8 @@ virDomainDeviceDefParse(const char *xmlStr,
goto error;
break;
case VIR_DOMAIN_DEVICE_CONTROLLER:
- if (!(dev->data.controller = virDomainControllerDefParseXML(node, ctxt,
- flags)))
+ if (!(dev->data.controller = virDomainControllerDefParseXML(xmlopt, node,
+ ctxt, flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_GRAPHICS:
@@ -15123,7 +15146,7 @@ virDomainDeviceDefParse(const char *xmlStr,
goto error;
break;
case VIR_DOMAIN_DEVICE_HUB:
- if (!(dev->data.hub = virDomainHubDefParseXML(node, flags)))
+ if (!(dev->data.hub = virDomainHubDefParseXML(xmlopt, node, flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_REDIRDEV:
@@ -15151,29 +15174,32 @@ virDomainDeviceDefParse(const char *xmlStr,
goto error;
break;
case VIR_DOMAIN_DEVICE_MEMBALLOON:
- if (!(dev->data.memballoon = virDomainMemballoonDefParseXML(node,
+ if (!(dev->data.memballoon = virDomainMemballoonDefParseXML(xmlopt,
+ node,
ctxt,
flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_NVRAM:
- if (!(dev->data.nvram = virDomainNVRAMDefParseXML(node, flags)))
+ if (!(dev->data.nvram = virDomainNVRAMDefParseXML(xmlopt, node, flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_SHMEM:
- if (!(dev->data.shmem = virDomainShmemDefParseXML(node, ctxt, flags)))
+ if (!(dev->data.shmem = virDomainShmemDefParseXML(xmlopt, node,
+ ctxt, flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_TPM:
- if (!(dev->data.tpm = virDomainTPMDefParseXML(node, ctxt, flags)))
+ if (!(dev->data.tpm = virDomainTPMDefParseXML(xmlopt, node, ctxt, flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_PANIC:
- if (!(dev->data.panic = virDomainPanicDefParseXML(node, flags)))
+ if (!(dev->data.panic = virDomainPanicDefParseXML(xmlopt, node, flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_MEMORY:
- if (!(dev->data.memory = virDomainMemoryDefParseXML(node, ctxt, flags)))
+ if (!(dev->data.memory = virDomainMemoryDefParseXML(xmlopt, node,
+ ctxt, flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_IOMMU:
@@ -18750,7 +18776,8 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
for (i = 0; i < n; i++) {
- virDomainControllerDefPtr controller = virDomainControllerDefParseXML(nodes[i],
+ virDomainControllerDefPtr controller = virDomainControllerDefParseXML(xmlopt,
+ nodes[i],
ctxt,
flags);
@@ -18816,7 +18843,9 @@ virDomainDefParseXML(xmlDocPtr xml,
if (n && VIR_ALLOC_N(def->fss, n) < 0)
goto error;
for (i = 0; i < n; i++) {
- virDomainFSDefPtr fs = virDomainFSDefParseXML(nodes[i], ctxt,
+ virDomainFSDefPtr fs = virDomainFSDefParseXML(xmlopt,
+ nodes[i],
+ ctxt,
flags);
if (!fs)
goto error;
@@ -18980,7 +19009,8 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
for (i = 0; i < n; i++) {
- virDomainInputDefPtr input = virDomainInputDefParseXML(def,
+ virDomainInputDefPtr input = virDomainInputDefParseXML(xmlopt,
+ def,
nodes[i],
ctxt,
flags);
@@ -19022,7 +19052,8 @@ virDomainDefParseXML(xmlDocPtr xml,
if (n && VIR_ALLOC_N(def->sounds, n) < 0)
goto error;
for (i = 0; i < n; i++) {
- virDomainSoundDefPtr sound = virDomainSoundDefParseXML(nodes[i],
+ virDomainSoundDefPtr sound = virDomainSoundDefParseXML(xmlopt,
+ nodes[i],
ctxt,
flags);
if (!sound)
@@ -19041,7 +19072,8 @@ virDomainDefParseXML(xmlDocPtr xml,
virDomainVideoDefPtr video;
ssize_t insertAt = -1;
- if (!(video = virDomainVideoDefParseXML(nodes[i], ctxt, def, flags)))
+ if (!(video = virDomainVideoDefParseXML(xmlopt, nodes[i],
+ ctxt, def, flags)))
goto error;
if (video->primary) {
@@ -19110,8 +19142,9 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}
if (n > 0) {
- virDomainWatchdogDefPtr watchdog =
- virDomainWatchdogDefParseXML(nodes[0], flags);
+ virDomainWatchdogDefPtr watchdog;
+
+ watchdog = virDomainWatchdogDefParseXML(xmlopt, nodes[0], flags);
if (!watchdog)
goto error;
@@ -19129,8 +19162,9 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}
if (n > 0) {
- virDomainMemballoonDefPtr memballoon =
- virDomainMemballoonDefParseXML(nodes[0], ctxt, flags);
+ virDomainMemballoonDefPtr memballoon;
+
+ memballoon = virDomainMemballoonDefParseXML(xmlopt, nodes[0], ctxt, flags);
if (!memballoon)
goto error;
@@ -19164,7 +19198,7 @@ virDomainDefParseXML(xmlDocPtr xml,
}
if (n > 0) {
- if (!(def->tpm = virDomainTPMDefParseXML(nodes[0], ctxt, flags)))
+ if (!(def->tpm = virDomainTPMDefParseXML(xmlopt, nodes[0], ctxt, flags)))
goto error;
}
VIR_FREE(nodes);
@@ -19178,7 +19212,7 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
} else if (n == 1) {
virDomainNVRAMDefPtr nvram =
- virDomainNVRAMDefParseXML(nodes[0], flags);
+ virDomainNVRAMDefParseXML(xmlopt, nodes[0], flags);
if (!nvram)
goto error;
def->nvram = nvram;
@@ -19191,7 +19225,9 @@ virDomainDefParseXML(xmlDocPtr xml,
if (n && VIR_ALLOC_N(def->hubs, n) < 0)
goto error;
for (i = 0; i < n; i++) {
- virDomainHubDefPtr hub = virDomainHubDefParseXML(nodes[i], flags);
+ virDomainHubDefPtr hub;
+
+ hub = virDomainHubDefParseXML(xmlopt, nodes[i], flags);
if (!hub)
goto error;
@@ -19247,7 +19283,9 @@ virDomainDefParseXML(xmlDocPtr xml,
if (n && VIR_ALLOC_N(def->panics, n) < 0)
goto error;
for (i = 0; i < n; i++) {
- virDomainPanicDefPtr panic = virDomainPanicDefParseXML(nodes[i], flags);
+ virDomainPanicDefPtr panic;
+
+ panic = virDomainPanicDefParseXML(xmlopt, nodes[i], flags);
if (!panic)
goto error;
@@ -19265,7 +19303,7 @@ virDomainDefParseXML(xmlDocPtr xml,
for (i = 0; i < n; i++) {
virDomainShmemDefPtr shmem;
ctxt->node = nodes[i];
- shmem = virDomainShmemDefParseXML(nodes[i], ctxt, flags);
+ shmem = virDomainShmemDefParseXML(xmlopt, nodes[i], ctxt, flags);
if (!shmem)
goto error;
@@ -19281,7 +19319,8 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
for (i = 0; i < n; i++) {
- virDomainMemoryDefPtr mem = virDomainMemoryDefParseXML(nodes[i],
+ virDomainMemoryDefPtr mem = virDomainMemoryDefParseXML(xmlopt,
+ nodes[i],
ctxt,
flags);
if (!mem)
--
2.13.6
3:11 a.m.
New subject: [libvirt] [PATCH v1 07/14] conf: Parse user supplied aliases
If driver that is calling the parse supports user supplied
aliases, they can be parsed even for inactive XMLs. However, to
avoid any clashes with aliases that libvirt generates, the user
ones have to have "ua-" prefix.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 13 +++++++++++--
src/conf/domain_conf.h | 1 +
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f4de4e288..f1386c116 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6398,6 +6398,10 @@ virDomainDeviceAddressParseXML(xmlNodePtr address,
}
+#define USER_ALIAS_PREFIX "ua-"
+#define USER_ALIAS_CHARS \
+ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-"
+
/* Parse the XML definition for a device address
* @param node XML nodeset to parse for device address definition
*/
@@ -6424,7 +6428,6 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt
ATTRIBUTE_UNUSED,
while (cur != NULL) {
if (cur->type == XML_ELEMENT_NODE) {
if (alias == NULL &&
- !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
virXMLNodeNameEqual(cur, "alias")) {
alias = cur;
} else if (address == NULL &&
@@ -6446,8 +6449,14 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt
ATTRIBUTE_UNUSED,
cur = cur->next;
}
- if (alias)
+ if (alias) {
info->alias = virXMLPropString(alias, "name");
+ if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE &&
+ (!STRPREFIX(info->alias, USER_ALIAS_PREFIX) ||
+ !(xmlopt->config.features & VIR_DOMAIN_DEF_FEATURE_USER_ALIAS) ||
+ strspn(info->alias, USER_ALIAS_CHARS) < strlen(info->alias)))
+ VIR_FREE(info->alias);
+ }
if (master) {
info->mastertype = VIR_DOMAIN_CONTROLLER_MASTER_USB;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1e007346f..5ff85057f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2508,6 +2508,7 @@ typedef enum {
VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN = (1 << 2),
VIR_DOMAIN_DEF_FEATURE_NAME_SLASH = (1 << 3),
VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS = (1 << 4),
+ VIR_DOMAIN_DEF_FEATURE_USER_ALIAS = (1 << 5),
} virDomainDefFeatures;
--
2.13.6
4:55 a.m.
New subject: [libvirt] [PATCH v1 07/14] conf: Parse user supplied aliases
On Thu, Oct 19, 2017 at 10:11:02AM +0200, Michal Privoznik wrote:
If driver that is calling the parse supports user supplied
aliases, they can be parsed even for inactive XMLs. However, to
avoid any clashes with aliases that libvirt generates, the user
ones have to have "ua-" prefix.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 13 +++++++++++--
src/conf/domain_conf.h | 1 +
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f4de4e288..f1386c116 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6398,6 +6398,10 @@ virDomainDeviceAddressParseXML(xmlNodePtr address,
}
+#define USER_ALIAS_PREFIX "ua-"
+#define USER_ALIAS_CHARS \
+ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-"
+
/* Parse the XML definition for a device address
* @param node XML nodeset to parse for device address definition
*/
@@ -6424,7 +6428,6 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt
ATTRIBUTE_UNUSED,
while (cur != NULL) {
if (cur->type == XML_ELEMENT_NODE) {
if (alias == NULL &&
- !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
virXMLNodeNameEqual(cur, "alias")) {
alias = cur;
} else if (address == NULL &&
@@ -6446,8 +6449,14 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt
ATTRIBUTE_UNUSED,
cur = cur->next;
}
- if (alias)
+ if (alias) {
info->alias = virXMLPropString(alias, "name");
+ if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE &&
+ (!STRPREFIX(info->alias, USER_ALIAS_PREFIX) ||
+ !(xmlopt->config.features & VIR_DOMAIN_DEF_FEATURE_USER_ALIAS) ||
+ strspn(info->alias, USER_ALIAS_CHARS) < strlen(info->alias)))
+ VIR_FREE(info->alias);
Assigning the pointer just so you can clear it after some hard to read condition
seems just plain wrong. Only assign it if the condition is false.
+ }
if (master) {
info->mastertype = VIR_DOMAIN_CONTROLLER_MASTER_USB;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1e007346f..5ff85057f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2508,6 +2508,7 @@ typedef enum {
VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN = (1 << 2),
VIR_DOMAIN_DEF_FEATURE_NAME_SLASH = (1 << 3),
VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS = (1 << 4),
+ VIR_DOMAIN_DEF_FEATURE_USER_ALIAS = (1 << 5),
Can you elaborate on why is this not the default and whydo we need to have a
flag for it? If it's just so that individual drivers can decide this, adding
this to the commit message is enough and ACK with this and the above fixed.
Also ACK to previous patches.
} virDomainDefFeatures;
--
2.13.6
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
10:34 a.m.
New subject: [libvirt] [PATCH v1 07/14] conf: Parse user supplied aliases
On 10/19/2017 11:55 AM, Martin Kletzander wrote:
On Thu, Oct 19, 2017 at 10:11:02AM +0200, Michal Privoznik wrote:
> If driver that is calling the parse supports user supplied
> aliases, they can be parsed even for inactive XMLs. However, to
> avoid any clashes with aliases that libvirt generates, the user
> ones have to have "ua-" prefix.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/conf/domain_conf.c | 13 +++++++++++--
> src/conf/domain_conf.h | 1 +
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index f4de4e288..f1386c116 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6398,6 +6398,10 @@ virDomainDeviceAddressParseXML(xmlNodePtr address,
> }
>
>
> +#define USER_ALIAS_PREFIX "ua-"
> +#define USER_ALIAS_CHARS \
> + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-"
> +
> /* Parse the XML definition for a device address
> * @param node XML nodeset to parse for device address definition
> */
> @@ -6424,7 +6428,6 @@
> virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt
> ATTRIBUTE_UNUSED,
> while (cur != NULL) {
> if (cur->type == XML_ELEMENT_NODE) {
> if (alias == NULL &&
> - !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
> virXMLNodeNameEqual(cur, "alias")) {
> alias = cur;
> } else if (address == NULL &&
> @@ -6446,8 +6449,14 @@
> virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt
> ATTRIBUTE_UNUSED,
> cur = cur->next;
> }
>
> - if (alias)
> + if (alias) {
> info->alias = virXMLPropString(alias, "name");
> + if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE &&
> + (!STRPREFIX(info->alias, USER_ALIAS_PREFIX) ||
> + !(xmlopt->config.features &
> VIR_DOMAIN_DEF_FEATURE_USER_ALIAS) ||
> + strspn(info->alias, USER_ALIAS_CHARS) <
> strlen(info->alias)))
> + VIR_FREE(info->alias);
Assigning the pointer just so you can clear it after some hard to read
condition
seems just plain wrong. Only assign it if the condition is false.
> + }
>
> if (master) {
> info->mastertype = VIR_DOMAIN_CONTROLLER_MASTER_USB;
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 1e007346f..5ff85057f 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2508,6 +2508,7 @@ typedef enum {
> VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN = (1 << 2),
> VIR_DOMAIN_DEF_FEATURE_NAME_SLASH = (1 << 3),
> VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS = (1 << 4),
> + VIR_DOMAIN_DEF_FEATURE_USER_ALIAS = (1 << 5),
Can you elaborate on why is this not the default and whydo we need to
have a
flag for it? If it's just so that individual drivers can decide this,
adding
this to the commit message is enough and ACK with this and the above fixed.
I've described the reasons in cover letter. In short, rather than
enabling this for all drivers, we can take the safe approach and enable
it just for the drivers we know are safe. For now. Also, not every
driver we have has notion of aliases. But if it turns out that we have
enabled it for all the drivers, this flag can be dropped. Until then,
I'd like to be rather safe than sorry.
Due to Pavel's findings (Thanks! Nice catch), I will send v2 (enhanced
with news.xml entry and tests). Meanwhile, I'm pushing 02-06
Michal
3:11 a.m.
New subject: [libvirt] [PATCH v1 08/14] conf: Validate user supplied aliases
They have to be unique within the domain. As usual, backwards
compatibility takes its price. In this particular situation we
have a device that is represented twice in a domain and so is its
alias.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++-
src/conf/domain_conf.h | 5 ++
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 3 +
4 files changed, 155 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f1386c116..0cf67dff1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5458,6 +5458,145 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
}
+struct virDomainDefValidateAliasesData {
+ virHashTablePtr aliases;
+};
+
+
+static int
+virDomainDeviceDefValidateAliasesIterator(virDomainDefPtr def,
+ virDomainDeviceDefPtr dev,
+ virDomainDeviceInfoPtr info,
+ void *opaque)
+{
+ struct virDomainDefValidateAliasesData *data = opaque;
+ const char *alias = info->alias;
+
+ if (!alias)
+ return 0;
+
+ /* Some crazy backcompat for consoles. */
+ if (def->nserials && def->nconsoles &&
+ def->consoles[0]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE
&&
+ def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL
&&
+ dev->type == VIR_DOMAIN_DEVICE_CHR &&
+ virDomainChrEquals(def->serials[0], dev->data.chr))
+ return 0;
+
+ if (virHashLookup(data->aliases, alias)) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("non unique alias detected: %s"),
+ alias);
+ return -1;
+ }
+
+ if (virHashAddEntry(data->aliases, alias, (void *) 1) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to construct table of device aliases"));
+ return -1;
+ }
+
+ return 0;
+}
+
+
+/**
+ * virDomainDefValidateAliases:
+ *
+ * Check for uniqueness of device aliases. If @aliases is not
+ * NULL return hash table of all the aliases in it.
+ *
+ * Returns 0 on success,
+ * -1 otherwise (with error reported).
+ */
+static int
+virDomainDefValidateAliases(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED,
+ const virDomainDef *def,
+ virHashTablePtr *aliases,
+ unsigned int parseFlags)
+{
+ struct virDomainDefValidateAliasesData data;
+ int ret = -1;
+
+ /* validate configuration only in certain places */
+ if (parseFlags & VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)
+ return 0;
+
+ /* We are not storing copies of aliases. Don't free them. */
+ if (!(data.aliases = virHashCreate(10, NULL)))
+ goto cleanup;
+
+ if (virDomainDeviceInfoIterateInternal((virDomainDefPtr) def,
+ virDomainDeviceDefValidateAliasesIterator,
+ true, &data) < 0)
+ goto cleanup;
+
+ if (aliases) {
+ *aliases = data.aliases;
+ data.aliases = NULL;
+ }
+
+ ret = 0;
+ cleanup:
+ virHashFree(data.aliases);
+ return ret;
+}
+
+
+static int
+virDomainDeviceValidateAliasImpl(virDomainXMLOptionPtr xmlopt,
+ const virDomainDef *def,
+ const virDomainDeviceDef *dev)
+{
+ virHashTablePtr aliases = NULL;
+ virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev);
+ int ret = -1;
+
+ if (!info || !info->alias)
+ return 0;
+
+ if (virDomainDefValidateAliases(xmlopt, def, &aliases, 0) < 0)
+ goto cleanup;
+
+ if (virHashLookup(aliases, info->alias)) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("non unique alias detected: %s"),
+ info->alias);
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
+
+ virHashFree(aliases);
+ return ret;
+}
+
+
+int
+virDomainDeviceValidateAliasForHotplug(virDomainXMLOptionPtr xmlopt,
+ virDomainObjPtr vm,
+ const virDomainDeviceDef *dev,
+ unsigned int flags)
+{
+ virDomainDefPtr persDef = NULL;
+ virDomainDefPtr liveDef = NULL;
+
+ if (virDomainObjGetDefs(vm, flags, &liveDef, &persDef) < 0)
+ return -1;
+
+ if (persDef &&
+ virDomainDeviceValidateAliasImpl(xmlopt, vm->def, dev) < 0)
+ return -1;
+
+ if (liveDef &&
+ virDomainDeviceValidateAliasImpl(xmlopt, vm->newDef, dev) < 0)
+ return -1;
+
+ return 0;
+}
+
+
static int
virDomainDeviceDefValidate(const virDomainDeviceDef *dev,
const virDomainDef *def,
@@ -5615,7 +5754,9 @@ virDomainDefCheckDuplicateDriveAddresses(const virDomainDef *def)
static int
-virDomainDefValidateInternal(const virDomainDef *def)
+virDomainDefValidateInternal(virDomainXMLOptionPtr xmlopt,
+ const virDomainDef *def,
+ unsigned int parseFlags)
{
if (virDomainDefCheckDuplicateDiskInfo(def) < 0)
return -1;
@@ -5626,6 +5767,9 @@ virDomainDefValidateInternal(const virDomainDef *def)
if (virDomainDefGetVcpusTopology(def, NULL) < 0)
return -1;
+ if (virDomainDefValidateAliases(xmlopt, def, NULL, parseFlags) < 0)
+ return -1;
+
if (def->iommu &&
def->iommu->intremap == VIR_TRISTATE_SWITCH_ON &&
(def->features[VIR_DOMAIN_FEATURE_IOAPIC] != VIR_TRISTATE_SWITCH_ON ||
@@ -5690,7 +5834,7 @@ virDomainDefValidate(virDomainDefPtr def,
true, &data) < 0)
return -1;
- if (virDomainDefValidateInternal(def) < 0)
+ if (virDomainDefValidateInternal(xmlopt, def, parseFlags) < 0)
return -1;
return 0;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5ff85057f..2b90e98cb 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2659,6 +2659,11 @@ int virDomainDefPostParse(virDomainDefPtr def,
virDomainXMLOptionPtr xmlopt,
void *parseOpaque);
+int virDomainDeviceValidateAliasForHotplug(virDomainXMLOptionPtr xmlopt,
+ virDomainObjPtr vm,
+ const virDomainDeviceDef *dev,
+ unsigned int flags);
+
int virDomainDefValidate(virDomainDefPtr def,
virCapsPtr caps,
unsigned int parseFlags,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3b5df28e5..8449e9685 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -294,6 +294,7 @@ virDomainDeviceFindControllerModel;
virDomainDeviceGetInfo;
virDomainDeviceInfoIterate;
virDomainDeviceTypeToString;
+virDomainDeviceValidateAliasForHotplug;
virDomainDiskBusTypeToString;
virDomainDiskByAddress;
virDomainDiskByName;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fb4d72236..dcd2c2109 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8375,6 +8375,9 @@ qemuDomainAttachDeviceLiveAndConfig(virConnectPtr conn,
if (dev == NULL)
goto cleanup;
+ if (virDomainDeviceValidateAliasForHotplug(driver->xmlopt, vm, dev, flags) <
0)
+ goto cleanup;
+
if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
flags & VIR_DOMAIN_AFFECT_LIVE) {
/* If we are affecting both CONFIG and LIVE
--
2.13.6
3:11 a.m.
New subject: [libvirt] [PATCH v1 09/14] virDomainDeviceInfoCheckABIStability: Check for alias too
Since we'll be passing user's input onto qemu command line, we
have to make sure aliases don't change during migration and all
the other places where ABI is checked. Aliases are part of ABI
now.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0cf67dff1..cb80939af 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19870,6 +19870,13 @@ virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src,
return false;
}
+ if (STRNEQ_NULLABLE(src->alias, dst->alias)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Target device alias %s does not match source %s"),
+ NULLSTR(src->alias), NULLSTR(dst->alias));
+ return false;
+ }
+
switch ((virDomainDeviceAddressType) src->type) {
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
if (src->addr.pci.domain != dst->addr.pci.domain ||
--
2.13.6
7:16 a.m.
New subject: [libvirt] [PATCH v1 09/14] virDomainDeviceInfoCheckABIStability: Check for alias too
On Thu, Oct 19, 2017 at 10:11:04AM +0200, Michal Privoznik wrote:
Since we'll be passing user's input onto qemu command line,
we
have to make sure aliases don't change during migration and all
the other places where ABI is checked. Aliases are part of ABI
now.
I don't see how change of an alias would have an impact on the guest OS since it
is not visible at all. Is QEMU using the IDs to refer to objects in the
migrated data stream? AFAIK (actually not me, I just asked random stranger
about it) it does not (except the memory modules that Peter told us about in one
of previous series). I see no reason why we would disable the option to rename
a device when doing migration. We are reconstructing the command-line anyway.
Did migration fail for you when you tried this?
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0cf67dff1..cb80939af 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19870,6 +19870,13 @@ virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src,
return false;
}
+ if (STRNEQ_NULLABLE(src->alias, dst->alias)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Target device alias %s does not match source %s"),
+ NULLSTR(src->alias), NULLSTR(dst->alias));
+ return false;
+ }
+
switch ((virDomainDeviceAddressType) src->type) {
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
if (src->addr.pci.domain != dst->addr.pci.domain ||
--
2.13.6
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
7:27 a.m.
New subject: [libvirt] [PATCH v1 09/14] virDomainDeviceInfoCheckABIStability: Check for alias too
On Thu, Oct 19, 2017 at 02:16:41PM +0200, Martin Kletzander wrote:
On Thu, Oct 19, 2017 at 10:11:04AM +0200, Michal Privoznik wrote:
> Since we'll be passing user's input onto qemu command line, we
> have to make sure aliases don't change during migration and all
> the other places where ABI is checked. Aliases are part of ABI
> now.
>
I don't see how change of an alias would have an impact on the guest OS since it
is not visible at all. Is QEMU using the IDs to refer to objects in the
migrated data stream? AFAIK (actually not me, I just asked random stranger
about it) it does not (except the memory modules that Peter told us about in one
of previous series). I see no reason why we would disable the option to rename
a device when doing migration. We are reconstructing the command-line anyway.
Did migration fail for you when you tried this?
QEMU never includes the device ID in the migration stream, except for that
one screw-up wrt memory modules that we know about.
So technically the change below could be loosened to only apply to the
memory module alias. Since the whole point of aliases is to have a long
term stable identifier for devices though, I figure it is probably
beneficial to enforce no-renames for all devices even if QEMU is safe
without it.
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/conf/domain_conf.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 0cf67dff1..cb80939af 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -19870,6 +19870,13 @@ virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr
src,
> return false;
> }
>
> + if (STRNEQ_NULLABLE(src->alias, dst->alias)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Target device alias %s does not match source
%s"),
> + NULLSTR(src->alias), NULLSTR(dst->alias));
> + return false;
> + }
> +
> switch ((virDomainDeviceAddressType) src->type) {
> case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
> if (src->addr.pci.domain != dst->addr.pci.domain ||
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
7:45 a.m.
New subject: [libvirt] [PATCH v1 09/14] virDomainDeviceInfoCheckABIStability: Check for alias too
On Thu, Oct 19, 2017 at 01:27:11PM +0100, Daniel P. Berrange wrote:
On Thu, Oct 19, 2017 at 02:16:41PM +0200, Martin Kletzander wrote:
> On Thu, Oct 19, 2017 at 10:11:04AM +0200, Michal Privoznik wrote:
> > Since we'll be passing user's input onto qemu command line, we
> > have to make sure aliases don't change during migration and all
> > the other places where ABI is checked. Aliases are part of ABI
> > now.
> >
>
> I don't see how change of an alias would have an impact on the guest OS since it
> is not visible at all. Is QEMU using the IDs to refer to objects in the
> migrated data stream? AFAIK (actually not me, I just asked random stranger
> about it) it does not (except the memory modules that Peter told us about in one
> of previous series). I see no reason why we would disable the option to rename
> a device when doing migration. We are reconstructing the command-line anyway.
> Did migration fail for you when you tried this?
QEMU never includes the device ID in the migration stream, except for that
one screw-up wrt memory modules that we know about.
So technically the change below could be loosened to only apply to the
memory module alias. Since the whole point of aliases is to have a long
term stable identifier for devices though, I figure it is probably
beneficial to enforce no-renames for all devices even if QEMU is safe
without it.
Well, depends how you define long-term. Since it is the user/mgmt app that
defines these aliases, I don't see why they could not change them.
>
> > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> > ---
> > src/conf/domain_conf.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 0cf67dff1..cb80939af 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -19870,6 +19870,13 @@
virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src,
> > return false;
> > }
> >
> > + if (STRNEQ_NULLABLE(src->alias, dst->alias)) {
> > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > + _("Target device alias %s does not match source
%s"),
> > + NULLSTR(src->alias), NULLSTR(dst->alias));
> > + return false;
> > + }
> > +
> > switch ((virDomainDeviceAddressType) src->type) {
> > case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
> > if (src->addr.pci.domain != dst->addr.pci.domain ||
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
3:11 a.m.
New subject: [libvirt] [PATCH v1 10/14] qemuxml2argvdata: Drop device aliases
The qemuxml2argvtest expects the domain XMLs to be inactive ones.
Therefore we should pass inactive XMLs.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.xml | 1 -
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftps.xml | 1 -
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.xml | 1 -
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-https.xml | 1 -
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-tftp.xml | 1 -
tests/qemuxml2argvdata/qemuxml2argv-usb-redir-filter.xml | 1 -
6 files changed, 6 deletions(-)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.xml
index df41e5832..b4e331160 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.xml
@@ -26,7 +26,6 @@
</source>
<target dev='hdc' bus='ide'/>
<readonly/>
- <alias name='ide0-1-0'/>
<address type='drive' controller='0' bus='1'
target='0' unit='0'/>
</disk>
<controller type='usb' index='0'/>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftps.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftps.xml
index 57049a266..4e6ca1bad 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftps.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftps.xml
@@ -26,7 +26,6 @@
</source>
<target dev='hdc' bus='ide'/>
<readonly/>
- <alias name='ide0-1-0'/>
<address type='drive' controller='0' bus='1'
target='0' unit='0'/>
</disk>
<controller type='usb' index='0'/>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.xml
index 8a1286cb1..0eed65672 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.xml
@@ -26,7 +26,6 @@
</source>
<target dev='hdc' bus='ide'/>
<readonly/>
- <alias name='ide0-1-0'/>
<address type='drive' controller='0' bus='1'
target='0' unit='0'/>
</disk>
<controller type='usb' index='0'/>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-https.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-https.xml
index 41b5a89b6..cd92fe44a 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-https.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-https.xml
@@ -26,7 +26,6 @@
</source>
<target dev='hdc' bus='ide'/>
<readonly/>
- <alias name='ide0-1-0'/>
<address type='drive' controller='0' bus='1'
target='0' unit='0'/>
</disk>
<controller type='usb' index='0'/>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-tftp.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-tftp.xml
index e2cb4a006..1c3b185b0 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-tftp.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-tftp.xml
@@ -26,7 +26,6 @@
</source>
<target dev='hdc' bus='ide'/>
<readonly/>
- <alias name='ide0-1-0'/>
<address type='drive' controller='0' bus='1'
target='0' unit='0'/>
</disk>
<controller type='usb' index='0'/>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-filter.xml
b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-filter.xml
index 32fb890a1..0bc15f0df 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-filter.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-filter.xml
@@ -33,7 +33,6 @@
<address type='usb' bus='0' port='4'/>
</redirdev>
<redirdev bus='usb' type='spicevmc'>
- <alias name='redir1'/>
<address type='usb' bus='0' port='5'/>
</redirdev>
<redirfilter>
--
2.13.6
3:11 a.m.
New subject: [libvirt] [PATCH v1 11/14] qemuhotplugtest: Load active XML
The point of this test is to load live XML and test hotplug. But
even though the XMLs we are parsing are live, the parsing is done
with VIR_DOMAIN_DEF_PARSE_INACTIVE flag.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/qemuhotplugtest.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index df28a7fbd..8d77c0056 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -62,6 +62,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
{
int ret = -1;
qemuDomainObjPrivatePtr priv = NULL;
+ const unsigned int parseFlags = 0;
if (!(*vm = virDomainObjNew(xmlopt)))
goto cleanup;
@@ -87,7 +88,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
driver.caps,
driver.xmlopt,
NULL,
- VIR_DOMAIN_DEF_PARSE_INACTIVE)))
+ parseFlags)))
goto cleanup;
if (qemuDomainAssignAddresses((*vm)->def, priv->qemuCaps,
--
2.13.6
3:11 a.m.
New subject: [libvirt] [PATCH v1 12/14] conf: Format alias even for inactive XMLs
We need to format alias even for inactive XMLs since that's the
way how users are going to identify their devices.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index cb80939af..7efbf55b0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5916,10 +5916,9 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
- if (info->alias &&
- !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) {
+
+ if (info->alias)
virBufferAsprintf(buf, "<alias name='%s'/>\n",
info->alias);
- }
if (info->mastertype == VIR_DOMAIN_CONTROLLER_MASTER_USB) {
virBufferAsprintf(buf, "<master startport='%d'/>\n",
--
2.13.6
3:11 a.m.
New subject: [libvirt] [PATCH v1 13/14] docs: Document user aliases
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
docs/formatdomain.html.in | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index b7e145e0f..57dd46dc5 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2249,6 +2249,29 @@
</dd>
</dl>
+ <p>
+ To help users identifying devices they care about, every
+ device can have direct child <code>alias</code> element
+ which then has <code>name</code> attribute where users can
+ store identifier for the device. The identifier has to have
+ "ua-" prefix and must be unique within the domain. Additionally, the
+ identifier must consist only of the following characters:
+ <code>[a-zA-Z0-9_-]</code>.
+ <span class="since">Since 3.9.0</span>
+ </p>
+
+<pre>
+<devices>
+ <disk type='file'>
+ <alias name='ua-myDisk'/>
+ </disk>
+ <interface type='network' trustGuestRxFilters='yes'>
+ <alias name='ua-myNIC'/>
+ </interface>
+ ...
+</devices>
+</pre>
+
<h4><a id="elementsDisks">Hard drives, floppy disks,
CDROMs</a></h4>
<p>
--
2.13.6
3:11 a.m.
New subject: [libvirt] [PATCH v1 14/14] qemu: Parse alias from inactive XMLs
https://bugzilla.redhat.com/show_bug.cgi?id=1434451
This way users can uniquely identify devices at define time.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_domain.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 05e8b96aa..0ddbea3b7 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4095,7 +4095,8 @@ virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = {
.features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG |
VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN |
- VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS,
+ VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS |
+ VIR_DOMAIN_DEF_FEATURE_USER_ALIAS,
};
--
2.13.6
8:33 a.m.
On Thu, Oct 19, 2017 at 10:10:55AM +0200, Michal Privoznik wrote:
As discussed earlier [1], we should allow users to set device
aliases at the define time. While the discussed approach calls
for generating missing aliases too, I'm saving that for another
patch set. There are couple of reasons for that:
a) I don't think it's really necessary (if users are interested
in a device they can just set the alias).
b) This patch set is already big enough as is.
c) Generating aliases is going to have its own problems.
Therefore, for now I'm only proposing parsing user supplied
aliases. The idea is that it's not enabled by default for all
drivers. Any driver that want to/can provide this feature has to
set VIR_DOMAIN_DEF_FEATURE_USER_ALIAS. Note that we have some
drivers that don't have notion of device aliases. But the code is
generic enough so that it should be easy to use in the drivers
that do know what aliases are.
This patch series missed some of the important parts of code
that relies on our generated aliases:
1. qemuGetNextChrDevIndex() ... this will fail while attaching new
chardev device without an alias if there is one already provided
by user and doesn't match the one that we generate.
2. qemuAssignDeviceRedirdevAlias() ... same as 1)
3. qemuAssignDeviceShmemAlias() ... same as 1)
4. qemuDomainNetVLAN() ... similar to the previous ones, hot-plugging
network interface with user alias will fail because the alias is used
to figure out VLAN of the interface.
Pavel
6:20 a.m.
On 10/19/2017 03:33 PM, Pavel Hrdina wrote:
On Thu, Oct 19, 2017 at 10:10:55AM +0200, Michal Privoznik wrote:
> As discussed earlier [1], we should allow users to set device
> aliases at the define time. While the discussed approach calls
> for generating missing aliases too, I'm saving that for another
> patch set. There are couple of reasons for that:
>
> a) I don't think it's really necessary (if users are interested
> in a device they can just set the alias).
>
> b) This patch set is already big enough as is.
>
> c) Generating aliases is going to have its own problems.
>
> Therefore, for now I'm only proposing parsing user supplied
> aliases. The idea is that it's not enabled by default for all
> drivers. Any driver that want to/can provide this feature has to
> set VIR_DOMAIN_DEF_FEATURE_USER_ALIAS. Note that we have some
> drivers that don't have notion of device aliases. But the code is
> generic enough so that it should be easy to use in the drivers
> that do know what aliases are.
This patch series missed some of the important parts of code
that relies on our generated aliases:
1. qemuGetNextChrDevIndex() ... this will fail while attaching new
chardev device without an alias if there is one already provided
by user and doesn't match the one that we generate.
2. qemuAssignDeviceRedirdevAlias() ... same as 1)
3. qemuAssignDeviceShmemAlias() ... same as 1)
Okay, these are easy to resolve.
4. qemuDomainNetVLAN() ... similar to the previous ones, hot-plugging
network interface with user alias will fail because the alias is used
to figure out VLAN of the interface.
This one. Well, this function is called only for ancient QEMUs (those
which lack QMP, and even not for all of them). So do we care? Sure, I
can cripple my code and allow user aliases only if running sufficiently
new QEMU. Or just shrug and walk away.
Michal
6:38 a.m.
On Fri, Oct 20, 2017 at 01:20:20PM +0200, Michal Privoznik wrote:
On 10/19/2017 03:33 PM, Pavel Hrdina wrote:
> On Thu, Oct 19, 2017 at 10:10:55AM +0200, Michal Privoznik wrote:
>> As discussed earlier [1], we should allow users to set device
>> aliases at the define time. While the discussed approach calls
>> for generating missing aliases too, I'm saving that for another
>> patch set. There are couple of reasons for that:
>>
>> a) I don't think it's really necessary (if users are interested
>> in a device they can just set the alias).
>>
>> b) This patch set is already big enough as is.
>>
>> c) Generating aliases is going to have its own problems.
>>
>> Therefore, for now I'm only proposing parsing user supplied
>> aliases. The idea is that it's not enabled by default for all
>> drivers. Any driver that want to/can provide this feature has to
>> set VIR_DOMAIN_DEF_FEATURE_USER_ALIAS. Note that we have some
>> drivers that don't have notion of device aliases. But the code is
>> generic enough so that it should be easy to use in the drivers
>> that do know what aliases are.
>
> This patch series missed some of the important parts of code
> that relies on our generated aliases:
>
> 1. qemuGetNextChrDevIndex() ... this will fail while attaching new
> chardev device without an alias if there is one already provided
> by user and doesn't match the one that we generate.
>
> 2. qemuAssignDeviceRedirdevAlias() ... same as 1)
>
> 3. qemuAssignDeviceShmemAlias() ... same as 1)
Okay, these are easy to resolve.
>
> 4. qemuDomainNetVLAN() ... similar to the previous ones, hot-plugging
> network interface with user alias will fail because the alias is used
> to figure out VLAN of the interface.
This one. Well, this function is called only for ancient QEMUs (those
which lack QMP, and even not for all of them). So do we care? Sure, I
can cripple my code and allow user aliases only if running sufficiently
new QEMU. Or just shrug and walk away.
In that case just ignore it, it will print an error message and since
this will affect only hot-plug of network devices with user alias
specified we can claim that this operation is unsupported.
Pavel
6:53 a.m.
On 10/20/2017 01:38 PM, Pavel Hrdina wrote:
On Fri, Oct 20, 2017 at 01:20:20PM +0200, Michal Privoznik wrote:
> On 10/19/2017 03:33 PM, Pavel Hrdina wrote:
>> On Thu, Oct 19, 2017 at 10:10:55AM +0200, Michal Privoznik wrote:
>>> As discussed earlier [1], we should allow users to set device
>>> aliases at the define time. While the discussed approach calls
>>> for generating missing aliases too, I'm saving that for another
>>> patch set. There are couple of reasons for that:
>>>
>>> a) I don't think it's really necessary (if users are interested
>>> in a device they can just set the alias).
>>>
>>> b) This patch set is already big enough as is.
>>>
>>> c) Generating aliases is going to have its own problems.
>>>
>>> Therefore, for now I'm only proposing parsing user supplied
>>> aliases. The idea is that it's not enabled by default for all
>>> drivers. Any driver that want to/can provide this feature has to
>>> set VIR_DOMAIN_DEF_FEATURE_USER_ALIAS. Note that we have some
>>> drivers that don't have notion of device aliases. But the code is
>>> generic enough so that it should be easy to use in the drivers
>>> that do know what aliases are.
>>
>> This patch series missed some of the important parts of code
>> that relies on our generated aliases:
>>
>> 1. qemuGetNextChrDevIndex() ... this will fail while attaching new
>> chardev device without an alias if there is one already provided
>> by user and doesn't match the one that we generate.
>>
>> 2. qemuAssignDeviceRedirdevAlias() ... same as 1)
>>
>> 3. qemuAssignDeviceShmemAlias() ... same as 1)
>
> Okay, these are easy to resolve.
>
>>
>> 4. qemuDomainNetVLAN() ... similar to the previous ones, hot-plugging
>> network interface with user alias will fail because the alias is used
>> to figure out VLAN of the interface.
>
> This one. Well, this function is called only for ancient QEMUs (those
> which lack QMP, and even not for all of them). So do we care? Sure, I
> can cripple my code and allow user aliases only if running sufficiently
> new QEMU. Or just shrug and walk away.
In that case just ignore it, it will print an error message and since
this will affect only hot-plug of network devices with user alias
specified we can claim that this operation is unsupported.
Yeah. Just don't forget the ancient QEMU part. So the whole story is
that if you're running QEMU that's lacking -netdev, and you want to
hotplug an interface with user supplied alias, we error out. Yeah, I'm
okay with that. We have to draw a line somewhere and say: yeah, it's
probably bad behaviour, be we don't care. Patches are welcome.
Michal
2590
days inactive
2591
days old
26 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Martin Kletzander -
Michal Privoznik -
Pavel Hrdina