7:46 a.m.
A while ago I was looking at the code and got annoyed by the lack
of consistency, both internal and external. This series addresses
most of it.
Andrea Bolognani (9):
qemu: Remove redundant condition
qemu: Use more specific prefixes
qemu: Move functions around
qemu: Add arch parameter to qemuDomainMachine*()
qemu: Add arch checks to qemuDomainMachine*()
qemu: Remove useless ARCH_IS_X86() call
qemu: Make most qemuDomainMachine*() functions static
qemu: Move qemuDomainSupportsPCI() to qemu_domain
qemu: Unify style for qemuDomain*()
src/qemu/qemu_capabilities.c | 10 +-
src/qemu/qemu_domain.c | 250 +++++++++++++++++++++------------
src/qemu/qemu_domain.h | 29 ++--
src/qemu/qemu_domain_address.c | 22 ---
4 files changed, 175 insertions(+), 136 deletions(-)
--
2.20.1
7:46 a.m.
New subject: [libvirt] [PATCH 1/9] qemu: Remove redundant condition
No need to check whether we're dealing with a pSeries guest
twice within just a few lines.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 81ef0357e7..7ccfcd054d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5224,8 +5224,10 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps,
VIR_DOMAIN_DISK_DEVICE_LUN);
/* PowerPC pseries based VMs do not support floppy device */
- if (!qemuDomainMachineIsPSeries(machine, qemuCaps->arch))
+ if (!qemuDomainMachineIsPSeries(machine, qemuCaps->arch)) {
VIR_DOMAIN_CAPS_ENUM_SET(disk->diskDevice, VIR_DOMAIN_DISK_DEVICE_FLOPPY);
+ VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_FDC);
+ }
if (qemuDomainMachineHasBuiltinIDE(machine))
VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_IDE);
@@ -5235,10 +5237,6 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps,
VIR_DOMAIN_DISK_BUS_VIRTIO,
/* VIR_DOMAIN_DISK_BUS_SD */);
- /* PowerPC pseries based VMs do not support floppy device */
- if (!qemuDomainMachineIsPSeries(machine, qemuCaps->arch))
- VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_FDC);
-
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE))
VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_USB);
--
2.20.1
7:46 a.m.
New subject: [libvirt] [PATCH 2/9] qemu: Use more specific prefixes
While the chances of the current checks resulting in false
positives are basically zero, it's still nicer to check for
the full prefix instead of the prefix's prefix.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b6c1a0e4e5..073f5cc86c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9880,7 +9880,7 @@ qemuDomainIsQ35(const virDomainDef *def)
bool
qemuDomainMachineIsQ35(const char *machine)
{
- return (STRPREFIX(machine, "pc-q35") ||
+ return (STRPREFIX(machine, "pc-q35-") ||
STREQ(machine, "q35"));
}
@@ -9898,7 +9898,7 @@ qemuDomainMachineIsI440FX(const char *machine)
return (STREQ(machine, "pc") ||
STRPREFIX(machine, "pc-0.") ||
STRPREFIX(machine, "pc-1.") ||
- STRPREFIX(machine, "pc-i440") ||
+ STRPREFIX(machine, "pc-i440fx-") ||
STRPREFIX(machine, "rhel"));
}
--
2.20.1
7:46 a.m.
New subject: [libvirt] [PATCH 3/9] qemu: Move functions around
Make sure related functions, eg. all qemuDomainIs*(), are
close together instead of being sprinkled throughout both
the header and implementation file, and also that all
qemuDomainMachine*() functions are declared first since
we're going to make a bunch of them static later on.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 148 ++++++++++++++++++++---------------------
src/qemu/qemu_domain.h | 24 +++----
2 files changed, 86 insertions(+), 86 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 073f5cc86c..bae49e97a4 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9870,13 +9870,6 @@ qemuFindAgentConfig(virDomainDefPtr def)
}
-bool
-qemuDomainIsQ35(const virDomainDef *def)
-{
- return qemuDomainMachineIsQ35(def->os.machine);
-}
-
-
bool
qemuDomainMachineIsQ35(const char *machine)
{
@@ -9885,13 +9878,6 @@ qemuDomainMachineIsQ35(const char *machine)
}
-bool
-qemuDomainIsI440FX(const virDomainDef *def)
-{
- return qemuDomainMachineIsI440FX(def->os.machine);
-}
-
-
bool
qemuDomainMachineIsI440FX(const char *machine)
{
@@ -9904,14 +9890,23 @@ qemuDomainMachineIsI440FX(const char *machine)
bool
-qemuDomainHasPCIRoot(const virDomainDef *def)
+qemuDomainMachineIsS390CCW(const char *machine)
{
- int root = virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0);
+ return STRPREFIX(machine, "s390-ccw");
+}
- if (root < 0)
+
+bool
+qemuDomainMachineIsARMVirt(const char *machine,
+ const virArch arch)
+{
+ if (arch != VIR_ARCH_ARMV6L &&
+ arch != VIR_ARCH_ARMV7L &&
+ arch != VIR_ARCH_AARCH64)
return false;
- if (def->controllers[root]->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
+ if (STRNEQ(machine, "virt") &&
+ !STRPREFIX(machine, "virt-"))
return false;
return true;
@@ -9919,14 +9914,29 @@ qemuDomainHasPCIRoot(const virDomainDef *def)
bool
-qemuDomainHasPCIeRoot(const virDomainDef *def)
+qemuDomainMachineIsRISCVVirt(const char *machine,
+ const virArch arch)
{
- int root = virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0);
+ if (!ARCH_IS_RISCV(arch))
+ return false;
- if (root < 0)
+ if (STRNEQ(machine, "virt") &&
+ !STRPREFIX(machine, "virt-"))
return false;
- if (def->controllers[root]->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
+ return true;
+}
+
+
+bool
+qemuDomainMachineIsPSeries(const char *machine,
+ const virArch arch)
+{
+ if (!ARCH_IS_PPC64(arch))
+ return false;
+
+ if (STRNEQ(machine, "pseries") &&
+ !STRPREFIX(machine, "pseries-"))
return false;
return true;
@@ -9934,9 +9944,12 @@ qemuDomainHasPCIeRoot(const virDomainDef *def)
bool
-qemuDomainNeedsFDC(const virDomainDef *def)
+qemuDomainMachineHasBuiltinIDE(const char *machine)
{
- return qemuDomainMachineNeedsFDC(def->os.machine);
+ return qemuDomainMachineIsI440FX(machine) ||
+ STREQ(machine, "malta") ||
+ STREQ(machine, "sun4u") ||
+ STREQ(machine, "g3beige");
}
@@ -9959,40 +9972,30 @@ qemuDomainMachineNeedsFDC(const char *machine)
bool
-qemuDomainIsS390CCW(const virDomainDef *def)
+qemuDomainIsQ35(const virDomainDef *def)
{
- return qemuDomainMachineIsS390CCW(def->os.machine);
+ return qemuDomainMachineIsQ35(def->os.machine);
}
bool
-qemuDomainMachineIsS390CCW(const char *machine)
+qemuDomainIsI440FX(const virDomainDef *def)
{
- return STRPREFIX(machine, "s390-ccw");
+ return qemuDomainMachineIsI440FX(def->os.machine);
}
bool
-qemuDomainIsARMVirt(const virDomainDef *def)
+qemuDomainIsS390CCW(const virDomainDef *def)
{
- return qemuDomainMachineIsARMVirt(def->os.machine, def->os.arch);
+ return qemuDomainMachineIsS390CCW(def->os.machine);
}
bool
-qemuDomainMachineIsARMVirt(const char *machine,
- const virArch arch)
+qemuDomainIsARMVirt(const virDomainDef *def)
{
- if (arch != VIR_ARCH_ARMV6L &&
- arch != VIR_ARCH_ARMV7L &&
- arch != VIR_ARCH_AARCH64)
- return false;
-
- if (STRNEQ(machine, "virt") &&
- !STRPREFIX(machine, "virt-"))
- return false;
-
- return true;
+ return qemuDomainMachineIsARMVirt(def->os.machine, def->os.arch);
}
@@ -10004,14 +10007,21 @@ qemuDomainIsRISCVVirt(const virDomainDef *def)
bool
-qemuDomainMachineIsRISCVVirt(const char *machine,
- const virArch arch)
+qemuDomainIsPSeries(const virDomainDef *def)
{
- if (!ARCH_IS_RISCV(arch))
+ return qemuDomainMachineIsPSeries(def->os.machine, def->os.arch);
+}
+
+
+bool
+qemuDomainHasPCIRoot(const virDomainDef *def)
+{
+ int root = virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0);
+
+ if (root < 0)
return false;
- if (STRNEQ(machine, "virt") &&
- !STRPREFIX(machine, "virt-"))
+ if (def->controllers[root]->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
return false;
return true;
@@ -10019,24 +10029,31 @@ qemuDomainMachineIsRISCVVirt(const char *machine,
bool
-qemuDomainIsPSeries(const virDomainDef *def)
+qemuDomainHasPCIeRoot(const virDomainDef *def)
{
- return qemuDomainMachineIsPSeries(def->os.machine, def->os.arch);
+ int root = virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0);
+
+ if (root < 0)
+ return false;
+
+ if (def->controllers[root]->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
+ return false;
+
+ return true;
}
bool
-qemuDomainMachineIsPSeries(const char *machine,
- const virArch arch)
+qemuDomainHasBuiltinIDE(const virDomainDef *def)
{
- if (!ARCH_IS_PPC64(arch))
- return false;
+ return qemuDomainMachineHasBuiltinIDE(def->os.machine);
+}
- if (STRNEQ(machine, "pseries") &&
- !STRPREFIX(machine, "pseries-"))
- return false;
- return true;
+bool
+qemuDomainNeedsFDC(const virDomainDef *def)
+{
+ return qemuDomainMachineNeedsFDC(def->os.machine);
}
@@ -10234,23 +10251,6 @@ qemuDomainDefValidateMemoryHotplug(const virDomainDef *def,
}
-bool
-qemuDomainHasBuiltinIDE(const virDomainDef *def)
-{
- return qemuDomainMachineHasBuiltinIDE(def->os.machine);
-}
-
-
-bool
-qemuDomainMachineHasBuiltinIDE(const char *machine)
-{
- return qemuDomainMachineIsI440FX(machine) ||
- STREQ(machine, "malta") ||
- STREQ(machine, "sun4u") ||
- STREQ(machine, "g3beige");
-}
-
-
/**
* qemuDomainUpdateCurrentMemorySize:
*
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index fe474170dc..0bd4408f12 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -828,20 +828,8 @@ void qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def,
virDomainChrDefPtr qemuFindAgentConfig(virDomainDefPtr def);
-bool qemuDomainIsQ35(const virDomainDef *def);
-bool qemuDomainIsI440FX(const virDomainDef *def);
-bool qemuDomainHasPCIRoot(const virDomainDef *def);
-bool qemuDomainHasPCIeRoot(const virDomainDef *def);
-bool qemuDomainNeedsFDC(const virDomainDef *def);
-bool qemuDomainIsS390CCW(const virDomainDef *def);
-bool qemuDomainIsARMVirt(const virDomainDef *def);
-bool qemuDomainIsRISCVVirt(const virDomainDef *def);
-bool qemuDomainIsPSeries(const virDomainDef *def);
-bool qemuDomainHasBuiltinIDE(const virDomainDef *def);
-
bool qemuDomainMachineIsQ35(const char *machine);
bool qemuDomainMachineIsI440FX(const char *machine);
-bool qemuDomainMachineNeedsFDC(const char *machine);
bool qemuDomainMachineIsS390CCW(const char *machine);
bool qemuDomainMachineIsARMVirt(const char *machine,
const virArch arch);
@@ -850,6 +838,18 @@ bool qemuDomainMachineIsRISCVVirt(const char *machine,
bool qemuDomainMachineIsPSeries(const char *machine,
const virArch arch);
bool qemuDomainMachineHasBuiltinIDE(const char *machine);
+bool qemuDomainMachineNeedsFDC(const char *machine);
+
+bool qemuDomainIsQ35(const virDomainDef *def);
+bool qemuDomainIsI440FX(const virDomainDef *def);
+bool qemuDomainIsS390CCW(const virDomainDef *def);
+bool qemuDomainIsARMVirt(const virDomainDef *def);
+bool qemuDomainIsRISCVVirt(const virDomainDef *def);
+bool qemuDomainIsPSeries(const virDomainDef *def);
+bool qemuDomainHasPCIRoot(const virDomainDef *def);
+bool qemuDomainHasPCIeRoot(const virDomainDef *def);
+bool qemuDomainHasBuiltinIDE(const virDomainDef *def);
+bool qemuDomainNeedsFDC(const virDomainDef *def);
int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
virDomainObjPtr vm);
--
2.20.1
7:46 a.m.
New subject: [libvirt] [PATCH 4/9] qemu: Add arch parameter to qemuDomainMachine*()
We want the signatures to be consistent, and also we're
going to start using the additional parameter next.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 2 +-
src/qemu/qemu_domain.c | 27 ++++++++++++++++-----------
src/qemu/qemu_domain.h | 15 ++++++++++-----
3 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 7ccfcd054d..496fa94375 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5229,7 +5229,7 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps,
VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_FDC);
}
- if (qemuDomainMachineHasBuiltinIDE(machine))
+ if (qemuDomainMachineHasBuiltinIDE(machine, qemuCaps->arch))
VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_IDE);
VIR_DOMAIN_CAPS_ENUM_SET(disk->bus,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bae49e97a4..41996583fc 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9871,7 +9871,8 @@ qemuFindAgentConfig(virDomainDefPtr def)
bool
-qemuDomainMachineIsQ35(const char *machine)
+qemuDomainMachineIsQ35(const char *machine,
+ const virArch arch ATTRIBUTE_UNUSED)
{
return (STRPREFIX(machine, "pc-q35-") ||
STREQ(machine, "q35"));
@@ -9879,7 +9880,8 @@ qemuDomainMachineIsQ35(const char *machine)
bool
-qemuDomainMachineIsI440FX(const char *machine)
+qemuDomainMachineIsI440FX(const char *machine,
+ const virArch arch ATTRIBUTE_UNUSED)
{
return (STREQ(machine, "pc") ||
STRPREFIX(machine, "pc-0.") ||
@@ -9890,7 +9892,8 @@ qemuDomainMachineIsI440FX(const char *machine)
bool
-qemuDomainMachineIsS390CCW(const char *machine)
+qemuDomainMachineIsS390CCW(const char *machine,
+ const virArch arch ATTRIBUTE_UNUSED)
{
return STRPREFIX(machine, "s390-ccw");
}
@@ -9944,9 +9947,10 @@ qemuDomainMachineIsPSeries(const char *machine,
bool
-qemuDomainMachineHasBuiltinIDE(const char *machine)
+qemuDomainMachineHasBuiltinIDE(const char *machine,
+ const virArch arch)
{
- return qemuDomainMachineIsI440FX(machine) ||
+ return qemuDomainMachineIsI440FX(machine, arch) ||
STREQ(machine, "malta") ||
STREQ(machine, "sun4u") ||
STREQ(machine, "g3beige");
@@ -9954,7 +9958,8 @@ qemuDomainMachineHasBuiltinIDE(const char *machine)
bool
-qemuDomainMachineNeedsFDC(const char *machine)
+qemuDomainMachineNeedsFDC(const char *machine,
+ const virArch arch ATTRIBUTE_UNUSED)
{
const char *p = STRSKIP(machine, "pc-q35-");
@@ -9974,21 +9979,21 @@ qemuDomainMachineNeedsFDC(const char *machine)
bool
qemuDomainIsQ35(const virDomainDef *def)
{
- return qemuDomainMachineIsQ35(def->os.machine);
+ return qemuDomainMachineIsQ35(def->os.machine, def->os.arch);
}
bool
qemuDomainIsI440FX(const virDomainDef *def)
{
- return qemuDomainMachineIsI440FX(def->os.machine);
+ return qemuDomainMachineIsI440FX(def->os.machine, def->os.arch);
}
bool
qemuDomainIsS390CCW(const virDomainDef *def)
{
- return qemuDomainMachineIsS390CCW(def->os.machine);
+ return qemuDomainMachineIsS390CCW(def->os.machine, def->os.arch);
}
@@ -10046,14 +10051,14 @@ qemuDomainHasPCIeRoot(const virDomainDef *def)
bool
qemuDomainHasBuiltinIDE(const virDomainDef *def)
{
- return qemuDomainMachineHasBuiltinIDE(def->os.machine);
+ return qemuDomainMachineHasBuiltinIDE(def->os.machine, def->os.arch);
}
bool
qemuDomainNeedsFDC(const virDomainDef *def)
{
- return qemuDomainMachineNeedsFDC(def->os.machine);
+ return qemuDomainMachineNeedsFDC(def->os.machine, def->os.arch);
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 0bd4408f12..e98999a026 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -828,17 +828,22 @@ void qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def,
virDomainChrDefPtr qemuFindAgentConfig(virDomainDefPtr def);
-bool qemuDomainMachineIsQ35(const char *machine);
-bool qemuDomainMachineIsI440FX(const char *machine);
-bool qemuDomainMachineIsS390CCW(const char *machine);
+bool qemuDomainMachineIsQ35(const char *machine,
+ const virArch arch);
+bool qemuDomainMachineIsI440FX(const char *machine,
+ const virArch arch);
+bool qemuDomainMachineIsS390CCW(const char *machine,
+ const virArch arch);
bool qemuDomainMachineIsARMVirt(const char *machine,
const virArch arch);
bool qemuDomainMachineIsRISCVVirt(const char *machine,
const virArch arch);
bool qemuDomainMachineIsPSeries(const char *machine,
const virArch arch);
-bool qemuDomainMachineHasBuiltinIDE(const char *machine);
-bool qemuDomainMachineNeedsFDC(const char *machine);
+bool qemuDomainMachineHasBuiltinIDE(const char *machine,
+ const virArch arch);
+bool qemuDomainMachineNeedsFDC(const char *machine,
+ const virArch arch);
bool qemuDomainIsQ35(const virDomainDef *def);
bool qemuDomainIsI440FX(const virDomainDef *def);
--
2.20.1
7:46 a.m.
New subject: [libvirt] [PATCH 5/9] qemu: Add arch checks to qemuDomainMachine*()
There is very little overlap in the machine types available
on different architectures, so broadly speaking checking the
machine type is usually enough; regardless, it's better to
check the architecture as well.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 41996583fc..5feb743ad1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9872,8 +9872,11 @@ qemuFindAgentConfig(virDomainDefPtr def)
bool
qemuDomainMachineIsQ35(const char *machine,
- const virArch arch ATTRIBUTE_UNUSED)
+ const virArch arch)
{
+ if (!ARCH_IS_X86(arch))
+ return false;
+
return (STRPREFIX(machine, "pc-q35-") ||
STREQ(machine, "q35"));
}
@@ -9881,8 +9884,11 @@ qemuDomainMachineIsQ35(const char *machine,
bool
qemuDomainMachineIsI440FX(const char *machine,
- const virArch arch ATTRIBUTE_UNUSED)
+ const virArch arch)
{
+ if (!ARCH_IS_X86(arch))
+ return false;
+
return (STREQ(machine, "pc") ||
STRPREFIX(machine, "pc-0.") ||
STRPREFIX(machine, "pc-1.") ||
@@ -9893,8 +9899,11 @@ qemuDomainMachineIsI440FX(const char *machine,
bool
qemuDomainMachineIsS390CCW(const char *machine,
- const virArch arch ATTRIBUTE_UNUSED)
+ const virArch arch)
{
+ if (!ARCH_IS_S390(arch))
+ return false;
+
return STRPREFIX(machine, "s390-ccw");
}
@@ -9959,10 +9968,13 @@ qemuDomainMachineHasBuiltinIDE(const char *machine,
bool
qemuDomainMachineNeedsFDC(const char *machine,
- const virArch arch ATTRIBUTE_UNUSED)
+ const virArch arch)
{
const char *p = STRSKIP(machine, "pc-q35-");
+ if (!ARCH_IS_X86(arch))
+ return false;
+
if (p) {
if (STRPREFIX(p, "1.") ||
STREQ(p, "2.0") ||
--
2.20.1
7:46 a.m.
New subject: [libvirt] [PATCH 6/9] qemu: Remove useless ARCH_IS_X86() call
Now that we have added architecture checks to all
qemuDomainIs*() functions, we no longer need to perform the
same checks separately.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 5feb743ad1..5820a4b515 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7775,7 +7775,7 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,
* because other architectures and machine types were introduced
* when libvirt already supported <controller type='usb'/>.
*/
- if (ARCH_IS_X86(def->os.arch) && qemuDomainIsI440FX(def) &&
+ if (qemuDomainIsI440FX(def) &&
usb && usb->idx == 0 &&
(usb->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT ||
usb->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI) &&
--
2.20.1
7:46 a.m.
New subject: [libvirt] [PATCH 7/9] qemu: Make most qemuDomainMachine*() functions static
Ideally we'd make all of them static, but there are a few
cases where we don't have a virDomainDef instance handy and
so they are the only option.
For the few ones we're forced to keep exporting, document
through comments that the alternative is preferred.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 16 +++++++++++-----
src/qemu/qemu_domain.h | 12 ++----------
2 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 5820a4b515..8db02f7738 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9870,7 +9870,7 @@ qemuFindAgentConfig(virDomainDefPtr def)
}
-bool
+static bool
qemuDomainMachineIsQ35(const char *machine,
const virArch arch)
{
@@ -9882,7 +9882,7 @@ qemuDomainMachineIsQ35(const char *machine,
}
-bool
+static bool
qemuDomainMachineIsI440FX(const char *machine,
const virArch arch)
{
@@ -9897,7 +9897,7 @@ qemuDomainMachineIsI440FX(const char *machine,
}
-bool
+static bool
qemuDomainMachineIsS390CCW(const char *machine,
const virArch arch)
{
@@ -9908,6 +9908,8 @@ qemuDomainMachineIsS390CCW(const char *machine,
}
+/* You should normally avoid this function and use
+ * qemuDomainIsARMVirt() instead. */
bool
qemuDomainMachineIsARMVirt(const char *machine,
const virArch arch)
@@ -9925,7 +9927,7 @@ qemuDomainMachineIsARMVirt(const char *machine,
}
-bool
+static bool
qemuDomainMachineIsRISCVVirt(const char *machine,
const virArch arch)
{
@@ -9940,6 +9942,8 @@ qemuDomainMachineIsRISCVVirt(const char *machine,
}
+/* You should normally avoid this function and use
+ * qemuDomainIsPSeries() instead. */
bool
qemuDomainMachineIsPSeries(const char *machine,
const virArch arch)
@@ -9955,6 +9959,8 @@ qemuDomainMachineIsPSeries(const char *machine,
}
+/* You should normally avoid this function and use
+ * qemuDomainHasBuiltinIDE() instead. */
bool
qemuDomainMachineHasBuiltinIDE(const char *machine,
const virArch arch)
@@ -9966,7 +9972,7 @@ qemuDomainMachineHasBuiltinIDE(const char *machine,
}
-bool
+static bool
qemuDomainMachineNeedsFDC(const char *machine,
const virArch arch)
{
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index e98999a026..89f45aeb2c 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -828,22 +828,14 @@ void qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def,
virDomainChrDefPtr qemuFindAgentConfig(virDomainDefPtr def);
-bool qemuDomainMachineIsQ35(const char *machine,
- const virArch arch);
-bool qemuDomainMachineIsI440FX(const char *machine,
- const virArch arch);
-bool qemuDomainMachineIsS390CCW(const char *machine,
- const virArch arch);
+/* You should normally avoid these functions and use the variant that
+ * doesn't have "Machine" in the name instead. */
bool qemuDomainMachineIsARMVirt(const char *machine,
const virArch arch);
-bool qemuDomainMachineIsRISCVVirt(const char *machine,
- const virArch arch);
bool qemuDomainMachineIsPSeries(const char *machine,
const virArch arch);
bool qemuDomainMachineHasBuiltinIDE(const char *machine,
const virArch arch);
-bool qemuDomainMachineNeedsFDC(const char *machine,
- const virArch arch);
bool qemuDomainIsQ35(const virDomainDef *def);
bool qemuDomainIsI440FX(const virDomainDef *def);
--
2.20.1
7:46 a.m.
New subject: [libvirt] [PATCH 8/9] qemu: Move qemuDomainSupportsPCI() to qemu_domain
The function operates on a virDomainDef and is not tied to
device address assignment in any way, so it makes more sense
for it to live along with qemuDomainIs*() and the like.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 22 ++++++++++++++++++++++
src/qemu/qemu_domain.h | 2 ++
src/qemu/qemu_domain_address.c | 22 ----------------------
3 files changed, 24 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8db02f7738..f222f78654 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -10080,6 +10080,28 @@ qemuDomainNeedsFDC(const virDomainDef *def)
}
+bool
+qemuDomainSupportsPCI(virDomainDefPtr def,
+ virQEMUCapsPtr qemuCaps)
+{
+ if ((def->os.arch != VIR_ARCH_ARMV6L) &&
+ (def->os.arch != VIR_ARCH_ARMV7L) &&
+ (def->os.arch != VIR_ARCH_AARCH64) &&
+ !ARCH_IS_RISCV(def->os.arch))
+ return true;
+
+ if (STREQ(def->os.machine, "versatilepb"))
+ return true;
+
+ if ((qemuDomainIsARMVirt(def) ||
+ qemuDomainIsRISCVVirt(def)) &&
+ virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX))
+ return true;
+
+ return false;
+}
+
+
static bool
qemuCheckMemoryDimmConflict(const virDomainDef *def,
const virDomainMemoryDef *mem)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 89f45aeb2c..f6b124e8fd 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -847,6 +847,8 @@ bool qemuDomainHasPCIRoot(const virDomainDef *def);
bool qemuDomainHasPCIeRoot(const virDomainDef *def);
bool qemuDomainHasBuiltinIDE(const virDomainDef *def);
bool qemuDomainNeedsFDC(const virDomainDef *def);
+bool qemuDomainSupportsPCI(virDomainDefPtr def,
+ virQEMUCapsPtr qemuCaps);
int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
virDomainObjPtr vm);
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index c35ecd8585..32fdd59566 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -2356,28 +2356,6 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
}
-static bool
-qemuDomainSupportsPCI(virDomainDefPtr def,
- virQEMUCapsPtr qemuCaps)
-{
- if ((def->os.arch != VIR_ARCH_ARMV6L) &&
- (def->os.arch != VIR_ARCH_ARMV7L) &&
- (def->os.arch != VIR_ARCH_AARCH64) &&
- !ARCH_IS_RISCV(def->os.arch))
- return true;
-
- if (STREQ(def->os.machine, "versatilepb"))
- return true;
-
- if ((qemuDomainIsARMVirt(def) ||
- qemuDomainIsRISCVVirt(def)) &&
- virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX))
- return true;
-
- return false;
-}
-
-
static void
qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont,
virDomainDefPtr def,
--
2.20.1
7:46 a.m.
New subject: [libvirt] [PATCH 9/9] qemu: Unify style for qemuDomain*()
These functions do mostly the same things, and it would be
preferrable if they did them in mostly the same ways. This
also fixes a few violations to our code style guidelines.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 89 +++++++++++++++++++++++++-----------------
1 file changed, 54 insertions(+), 35 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f222f78654..eb4de1e6bd 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9877,8 +9877,12 @@ qemuDomainMachineIsQ35(const char *machine,
if (!ARCH_IS_X86(arch))
return false;
- return (STRPREFIX(machine, "pc-q35-") ||
- STREQ(machine, "q35"));
+ if (STREQ(machine, "q35") ||
+ STRPREFIX(machine, "pc-q35-")) {
+ return true;
+ }
+
+ return false;
}
@@ -9889,11 +9893,15 @@ qemuDomainMachineIsI440FX(const char *machine,
if (!ARCH_IS_X86(arch))
return false;
- return (STREQ(machine, "pc") ||
- STRPREFIX(machine, "pc-0.") ||
- STRPREFIX(machine, "pc-1.") ||
- STRPREFIX(machine, "pc-i440fx-") ||
- STRPREFIX(machine, "rhel"));
+ if (STREQ(machine, "pc") ||
+ STRPREFIX(machine, "pc-0.") ||
+ STRPREFIX(machine, "pc-1.") ||
+ STRPREFIX(machine, "pc-i440fx-") ||
+ STRPREFIX(machine, "rhel")) {
+ return true;
+ }
+
+ return false;
}
@@ -9904,7 +9912,10 @@ qemuDomainMachineIsS390CCW(const char *machine,
if (!ARCH_IS_S390(arch))
return false;
- return STRPREFIX(machine, "s390-ccw");
+ if (STRPREFIX(machine, "s390-ccw"))
+ return true;
+
+ return false;
}
@@ -9916,14 +9927,16 @@ qemuDomainMachineIsARMVirt(const char *machine,
{
if (arch != VIR_ARCH_ARMV6L &&
arch != VIR_ARCH_ARMV7L &&
- arch != VIR_ARCH_AARCH64)
+ arch != VIR_ARCH_AARCH64) {
return false;
+ }
- if (STRNEQ(machine, "virt") &&
- !STRPREFIX(machine, "virt-"))
- return false;
+ if (STREQ(machine, "virt") ||
+ STRPREFIX(machine, "virt-")) {
+ return true;
+ }
- return true;
+ return false;
}
@@ -9934,11 +9947,12 @@ qemuDomainMachineIsRISCVVirt(const char *machine,
if (!ARCH_IS_RISCV(arch))
return false;
- if (STRNEQ(machine, "virt") &&
- !STRPREFIX(machine, "virt-"))
- return false;
+ if (STREQ(machine, "virt") ||
+ STRPREFIX(machine, "virt-")) {
+ return true;
+ }
- return true;
+ return false;
}
@@ -9951,11 +9965,12 @@ qemuDomainMachineIsPSeries(const char *machine,
if (!ARCH_IS_PPC64(arch))
return false;
- if (STRNEQ(machine, "pseries") &&
- !STRPREFIX(machine, "pseries-"))
- return false;
+ if (STREQ(machine, "pseries") ||
+ STRPREFIX(machine, "pseries-")) {
+ return true;
+ }
- return true;
+ return false;
}
@@ -9981,16 +9996,18 @@ qemuDomainMachineNeedsFDC(const char *machine,
if (!ARCH_IS_X86(arch))
return false;
- if (p) {
- if (STRPREFIX(p, "1.") ||
- STREQ(p, "2.0") ||
- STREQ(p, "2.1") ||
- STREQ(p, "2.2") ||
- STREQ(p, "2.3"))
- return false;
- return true;
+ if (!p)
+ return false;
+
+ if (STRPREFIX(p, "1.") ||
+ STREQ(p, "2.0") ||
+ STREQ(p, "2.1") ||
+ STREQ(p, "2.2") ||
+ STREQ(p, "2.3")) {
+ return false;
}
- return false;
+
+ return true;
}
@@ -10084,19 +10101,21 @@ bool
qemuDomainSupportsPCI(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps)
{
- if ((def->os.arch != VIR_ARCH_ARMV6L) &&
- (def->os.arch != VIR_ARCH_ARMV7L) &&
- (def->os.arch != VIR_ARCH_AARCH64) &&
- !ARCH_IS_RISCV(def->os.arch))
+ if (def->os.arch != VIR_ARCH_ARMV6L &&
+ def->os.arch != VIR_ARCH_ARMV7L &&
+ def->os.arch != VIR_ARCH_AARCH64 &&
+ !ARCH_IS_RISCV(def->os.arch)) {
return true;
+ }
if (STREQ(def->os.machine, "versatilepb"))
return true;
if ((qemuDomainIsARMVirt(def) ||
qemuDomainIsRISCVVirt(def)) &&
- virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX))
+ virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX)) {
return true;
+ }
return false;
}
--
2.20.1
7:18 a.m.
On 2/6/19 2:46 PM, Andrea Bolognani wrote:
A while ago I was looking at the code and got annoyed by the lack
of consistency, both internal and external. This series addresses
most of it.
Andrea Bolognani (9):
qemu: Remove redundant condition
qemu: Use more specific prefixes
qemu: Move functions around
qemu: Add arch parameter to qemuDomainMachine*()
qemu: Add arch checks to qemuDomainMachine*()
qemu: Remove useless ARCH_IS_X86() call
qemu: Make most qemuDomainMachine*() functions static
qemu: Move qemuDomainSupportsPCI() to qemu_domain
qemu: Unify style for qemuDomain*()
src/qemu/qemu_capabilities.c | 10 +-
src/qemu/qemu_domain.c | 250 +++++++++++++++++++++------------
src/qemu/qemu_domain.h | 29 ++--
src/qemu/qemu_domain_address.c | 22 ---
4 files changed, 175 insertions(+), 136 deletions(-)
ACK
Michal
2115
days inactive
2117
days old
10 comments
2 participants
participants (2)
-
Andrea Bolognani -
Michal Privoznik