1:35 p.m.
On 04/06/2016 12:02 PM, Peter Krempa wrote:
---
src/qemu/qemu_command.c | 3 +--
src/qemu/qemu_domain.c | 11 +++++++++--
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_driver.c | 12 ++++--------
src/qemu/qemu_process.c | 9 +++------
5 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a2448bf..1518a53 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3408,8 +3408,7 @@ qemuBuildMemballoonCommandLine(virCommandPtr cmd,
virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_S390) && def->memballoon)
def->memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_NONE;
- if (!def->memballoon ||
- def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE)
+ if (!qemuDomainDefHasMemballoon(def))
return 0;
if (def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f38b0f3..c13acd1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4218,6 +4218,14 @@ qemuDomainMachineHasBuiltinIDE(const virDomainDef *def)
}
+bool
+qemuDomainDefHasMemballoon(const virDomainDef *def)
+{
+ return def->memballoon &&
+ def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_NONE;
I guess I would think would be if balloon and model == MODEL_VIRTIO
I find it easier to read absolutes rather than considering alternatives
to the != check... in this case that's only MODEL_XEN which yes should
never happen, but with the && at least there's no question...
+}
+
+
/**
* qemuDomainUpdateCurrentMemorySize:
*
@@ -4242,8 +4250,7 @@ qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
/* if no balloning is available, the current size equals to the current
* full memory size */
- if (!vm->def->memballoon ||
- vm->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE) {
+ if (!qemuDomainDefHasMemballoon(vm->def)) {
vm->def->mem.cur_balloon = virDomainDefGetMemoryActual(vm->def);
return 0;
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 54d7bd7..2992214 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -516,6 +516,7 @@ bool qemuDomainHasBlockjob(virDomainObjPtr vm, bool copy_only)
int qemuDomainAlignMemorySizes(virDomainDefPtr def);
void qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def,
virDomainMemoryDefPtr mem);
+bool qemuDomainDefHasMemballoon(const virDomainDef *def);
virDomainChrDefPtr qemuFindAgentConfig(virDomainDefPtr def);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index eaabe58..a00268b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2505,8 +2505,7 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int
period,
priv = vm->privateData;
if (def) {
- if (!def->memballoon ||
- def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
This is what I was thinking - either this construct or the corollary
balloon && model == VIRTIO.
+ if (!qemuDomainDefHasMemballoon(def)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Memory balloon model must be virtio to set the"
" collection period"));
@@ -2529,8 +2528,7 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int
period,
}
if (persistentDef) {
- if (!persistentDef->memballoon ||
- persistentDef->memballoon->model !=
VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
+ if (!qemuDomainDefHasMemballoon(persistentDef)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Memory balloon model must be virtio to set the"
" collection period"));
@@ -11495,8 +11493,7 @@ qemuDomainMemoryStats(virDomainPtr dom,
goto endjob;
}
- if (vm->def->memballoon &&
- vm->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) {
+ if (qemuDomainDefHasMemballoon(vm->def)) {
priv = vm->privateData;
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorGetMemoryStats(priv->mon, stats, nr_stats);
@@ -19061,8 +19058,7 @@ qemuDomainGetStatsBalloon(virQEMUDriverPtr driver
ATTRIBUTE_UNUSED,
unsigned long long cur_balloon = 0;
int err = 0;
- if (dom->def->memballoon &&
- dom->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE) {
+ if (!qemuDomainDefHasMemballoon(dom->def)) {
Ugh.. the ugly duckling
This changes the statement from if we have a model=NONE memballoon to we
have a memballoon with a model that's not NONE. Double negatives.
I think the logic has the right thought, but would be perhaps clearer in
my mind with the adjusted logic in the common function.
I think this is ACK-able just want get your thoughts on the logic for
the helper...
John
cur_balloon = virDomainDefGetMemoryActual(dom->def);
} else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BALLOON_EVENT)) {
cur_balloon = dom->def->mem.cur_balloon;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d9dca74..4911c1d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1933,8 +1933,7 @@ qemuProcessRefreshBalloonState(virQEMUDriverPtr driver,
/* if no ballooning is available, the current size equals to the current
* full memory size */
- if (!vm->def->memballoon ||
- vm->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE) {
+ if (!qemuDomainDefHasMemballoon(vm->def)) {
vm->def->mem.cur_balloon = virDomainDefGetMemoryActual(vm->def);
return 0;
}
@@ -4382,8 +4381,7 @@ qemuProcessSetupBalloon(virQEMUDriverPtr driver,
int period;
int ret = -1;
- if (!vm->def->memballoon ||
- vm->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE)
+ if (!qemuDomainDefHasMemballoon(vm->def))
return 0;
period = vm->def->memballoon->period;
@@ -6237,8 +6235,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
if (running) {
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
VIR_DOMAIN_RUNNING_UNPAUSED);
- if (vm->def->memballoon &&
- vm->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO
&&
+ if (qemuDomainDefHasMemballoon(vm->def) &&
vm->def->memballoon->period) {
qemuDomainObjEnterMonitor(driver, vm);
qemuMonitorSetMemoryStatsPeriod(priv->mon,