5:45 a.m.
diff to v1:
* dropped 1 wrong commit (thanks Daniel)
* improved setting of the variable (suggested by Peter, Michal, Martin)
* extended the switch to include all cases instead of simple if to be
future proof (suggested by Peter)
v1:
https://listman.redhat.com/archives/libvir-list/2022-July/232991.html
Kristina Hanicova (8):
domain_capabilities: use early return in
virDomainCapsFeatureSEVFormat()
domain_capabilities: reformat virDomainCapsFeatureSEVFormat()
domain_capabilities: reformat virDomainCapsCPUCustomFormat()
domain_conf: remove breaks after return in
virDomainChrSourceDefIsEqual()
domain_conf: extend switch with if in virDomainChrDefFree()
domain_conf: use early return in virDomainObjAssignDef()
domain_conf: rewrite conditions in virDomainObjWaitUntil()
domain_conf: rewrite variable setting
src/conf/domain_capabilities.c | 37 ++++++++---------
src/conf/domain_conf.c | 76 +++++++++++++++++-----------------
2 files changed, 55 insertions(+), 58 deletions(-)
--
2.35.3
5:45 a.m.
New subject: [PATCH v2 1/8] domain_capabilities: use early return in virDomainCapsFeatureSEVFormat()
Signed-off-by: Kristina Hanicova <khanicov at redhat.com>
---
src/conf/domain_capabilities.c | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 33570a51db..bb36a956db 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -590,25 +590,24 @@ virDomainCapsFeatureSEVFormat(virBuffer *buf,
{
if (!sev) {
virBufferAddLit(buf, "<sev supported='no'/>\n");
- } else {
- virBufferAddLit(buf, "<sev supported='yes'>\n");
- virBufferAdjustIndent(buf, 2);
- virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n",
sev->cbitpos);
- virBufferAsprintf(buf,
"<reducedPhysBits>%d</reducedPhysBits>\n",
- sev->reduced_phys_bits);
- virBufferAsprintf(buf, "<maxGuests>%d</maxGuests>\n",
- sev->max_guests);
- virBufferAsprintf(buf, "<maxESGuests>%d</maxESGuests>\n",
- sev->max_es_guests);
- if (sev->cpu0_id != NULL) {
- virBufferAsprintf(buf, "<cpu0Id>%s</cpu0Id>\n",
- sev->cpu0_id);
- }
- virBufferAdjustIndent(buf, -2);
- virBufferAddLit(buf, "</sev>\n");
+ return;
}
- return;
+ virBufferAddLit(buf, "<sev supported='yes'>\n");
+ virBufferAdjustIndent(buf, 2);
+ virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n",
sev->cbitpos);
+ virBufferAsprintf(buf,
"<reducedPhysBits>%d</reducedPhysBits>\n",
+ sev->reduced_phys_bits);
+ virBufferAsprintf(buf, "<maxGuests>%d</maxGuests>\n",
+ sev->max_guests);
+ virBufferAsprintf(buf, "<maxESGuests>%d</maxESGuests>\n",
+ sev->max_es_guests);
+ if (sev->cpu0_id != NULL) {
+ virBufferAsprintf(buf, "<cpu0Id>%s</cpu0Id>\n",
+ sev->cpu0_id);
+ }
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</sev>\n");
}
--
2.35.3
9:21 a.m.
New subject: [PATCH v2 1/8] domain_capabilities: use early return in virDomainCapsFeatureSEVFormat()
On Thu, Jul 21, 2022 at 12:45:46PM +0200, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova <khanicov at redhat.com>
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
5:45 a.m.
New subject: [PATCH v2 2/8] domain_capabilities: reformat virDomainCapsFeatureSEVFormat()
Signed-off-by: Kristina Hanicova <khanicov at redhat.com>
---
src/conf/domain_capabilities.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index bb36a956db..6e8f957657 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -598,14 +598,12 @@ virDomainCapsFeatureSEVFormat(virBuffer *buf,
virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n",
sev->cbitpos);
virBufferAsprintf(buf,
"<reducedPhysBits>%d</reducedPhysBits>\n",
sev->reduced_phys_bits);
- virBufferAsprintf(buf, "<maxGuests>%d</maxGuests>\n",
- sev->max_guests);
- virBufferAsprintf(buf, "<maxESGuests>%d</maxESGuests>\n",
- sev->max_es_guests);
- if (sev->cpu0_id != NULL) {
- virBufferAsprintf(buf, "<cpu0Id>%s</cpu0Id>\n",
- sev->cpu0_id);
- }
+ virBufferAsprintf(buf, "<maxGuests>%d</maxGuests>\n",
sev->max_guests);
+ virBufferAsprintf(buf, "<maxESGuests>%d</maxESGuests>\n",
sev->max_es_guests);
+
+ if (sev->cpu0_id != NULL)
+ virBufferAsprintf(buf, "<cpu0Id>%s</cpu0Id>\n",
sev->cpu0_id);
+
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</sev>\n");
}
--
2.35.3
5:45 a.m.
New subject: [PATCH v2 3/8] domain_capabilities: reformat virDomainCapsCPUCustomFormat()
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_capabilities.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 6e8f957657..653123f293 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -376,12 +376,14 @@ virDomainCapsCPUCustomFormat(virBuffer *buf,
for (i = 0; i < custom->nmodels; i++) {
virDomainCapsCPUModel *model = custom->models + i;
+
virBufferAsprintf(buf, "<model usable='%s'",
virDomainCapsCPUUsableTypeToString(model->usable));
+
if (model->deprecated)
virBufferAddLit(buf, " deprecated='yes'");
- virBufferAsprintf(buf, ">%s</model>\n",
- model->name);
+
+ virBufferAsprintf(buf, ">%s</model>\n", model->name);
}
virBufferAdjustIndent(buf, -2);
--
2.35.3
9:55 a.m.
New subject: [PATCH v2 3/8] domain_capabilities: reformat virDomainCapsCPUCustomFormat()
On Thu, Jul 21, 2022 at 12:45:48PM +0200, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
I was thinking if this would look nicer with virXMLFormatElement, but
then I realised it's just a two lines of code and it does not matter at
all =)
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/conf/domain_capabilities.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 6e8f957657..653123f293 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -376,12 +376,14 @@ virDomainCapsCPUCustomFormat(virBuffer *buf,
for (i = 0; i < custom->nmodels; i++) {
virDomainCapsCPUModel *model = custom->models + i;
+
virBufferAsprintf(buf, "<model usable='%s'",
virDomainCapsCPUUsableTypeToString(model->usable));
+
if (model->deprecated)
virBufferAddLit(buf, " deprecated='yes'");
- virBufferAsprintf(buf, ">%s</model>\n",
- model->name);
+
+ virBufferAsprintf(buf, ">%s</model>\n", model->name);
}
virBufferAdjustIndent(buf, -2);
--
2.35.3
5:45 a.m.
New subject: [PATCH v2 4/8] domain_conf: remove breaks after return in virDomainChrSourceDefIsEqual()
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 26a241db38..b903dac1cb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2844,21 +2844,22 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
case VIR_DOMAIN_CHR_TYPE_FILE:
return src->data.file.append == tgt->data.file.append &&
STREQ_NULLABLE(src->data.file.path, tgt->data.file.path);
- break;
+
case VIR_DOMAIN_CHR_TYPE_PTY:
case VIR_DOMAIN_CHR_TYPE_DEV:
case VIR_DOMAIN_CHR_TYPE_PIPE:
return STREQ_NULLABLE(src->data.file.path, tgt->data.file.path);
+
case VIR_DOMAIN_CHR_TYPE_NMDM:
return STREQ_NULLABLE(src->data.nmdm.master, tgt->data.nmdm.master)
&&
STREQ_NULLABLE(src->data.nmdm.slave, tgt->data.nmdm.slave);
- break;
+
case VIR_DOMAIN_CHR_TYPE_UDP:
return STREQ_NULLABLE(src->data.udp.bindHost, tgt->data.udp.bindHost)
&&
STREQ_NULLABLE(src->data.udp.bindService, tgt->data.udp.bindService)
&&
STREQ_NULLABLE(src->data.udp.connectHost, tgt->data.udp.connectHost)
&&
STREQ_NULLABLE(src->data.udp.connectService,
tgt->data.udp.connectService);
- break;
+
case VIR_DOMAIN_CHR_TYPE_TCP:
return src->data.tcp.listen == tgt->data.tcp.listen &&
src->data.tcp.protocol == tgt->data.tcp.protocol &&
@@ -2866,18 +2867,16 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
STREQ_NULLABLE(src->data.tcp.service, tgt->data.tcp.service)
&&
src->data.tcp.reconnect.enabled == tgt->data.tcp.reconnect.enabled
&&
src->data.tcp.reconnect.timeout == tgt->data.tcp.reconnect.timeout;
- break;
+
case VIR_DOMAIN_CHR_TYPE_UNIX:
return src->data.nix.listen == tgt->data.nix.listen &&
STREQ_NULLABLE(src->data.nix.path, tgt->data.nix.path) &&
src->data.nix.reconnect.enabled == tgt->data.nix.reconnect.enabled
&&
src->data.nix.reconnect.timeout == tgt->data.nix.reconnect.timeout;
- break;
case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
return STREQ_NULLABLE(src->data.spiceport.channel,
tgt->data.spiceport.channel);
- break;
case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
return src->data.spicevmc == tgt->data.spicevmc;
@@ -2885,12 +2884,10 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
case VIR_DOMAIN_CHR_TYPE_QEMU_VDAGENT:
return src->data.qemuVdagent.clipboard == tgt->data.qemuVdagent.clipboard
&&
src->data.qemuVdagent.mouse == tgt->data.qemuVdagent.mouse;
- break;
case VIR_DOMAIN_CHR_TYPE_DBUS:
return STREQ_NULLABLE(src->data.dbus.channel,
tgt->data.dbus.channel);
- break;
case VIR_DOMAIN_CHR_TYPE_NULL:
case VIR_DOMAIN_CHR_TYPE_VC:
--
2.35.3
9:56 a.m.
New subject: [PATCH v2 4/8] domain_conf: remove breaks after return in virDomainChrSourceDefIsEqual()
On Thu, Jul 21, 2022 at 12:45:49PM +0200, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
5:45 a.m.
New subject: [PATCH v2 5/8] domain_conf: extend switch with if in virDomainChrDefFree()
Switch is used for just one case, but a more future proof
approach is to handle all enum values.
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b903dac1cb..41eb105a6c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2904,7 +2904,7 @@ void virDomainChrDefFree(virDomainChrDef *def)
if (!def)
return;
- switch (def->deviceType) {
+ switch ((virDomainChrDeviceType)def->deviceType) {
case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL:
switch (def->targetType) {
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
@@ -2918,7 +2918,10 @@ void virDomainChrDefFree(virDomainChrDef *def)
}
break;
- default:
+ case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL:
+ case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL:
+ case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
+ case VIR_DOMAIN_CHR_DEVICE_TYPE_LAST:
break;
}
--
2.35.3
10:04 a.m.
New subject: [PATCH v2 5/8] domain_conf: extend switch with if in virDomainChrDefFree()
sorry for the confusing commit message, it should have been "domain_conf:
extend switch in virDomainChrDefFree()" only:D
Kristina
On Thu, Jul 21, 2022 at 12:46 PM Kristina Hanicova <khanicov(a)redhat.com>
wrote:
Switch is used for just one case, but a more future proof
approach is to handle all enum values.
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b903dac1cb..41eb105a6c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2904,7 +2904,7 @@ void virDomainChrDefFree(virDomainChrDef *def)
if (!def)
return;
- switch (def->deviceType) {
+ switch ((virDomainChrDeviceType)def->deviceType) {
case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL:
switch (def->targetType) {
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
@@ -2918,7 +2918,10 @@ void virDomainChrDefFree(virDomainChrDef *def)
}
break;
- default:
+ case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL:
+ case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL:
+ case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
+ case VIR_DOMAIN_CHR_DEVICE_TYPE_LAST:
break;
}
--
2.35.3
10:08 a.m.
New subject: [PATCH v2 5/8] domain_conf: extend switch with if in virDomainChrDefFree()
On Thu, Jul 21, 2022 at 05:04:43PM +0200, Kristina Hanicova wrote:
sorry for the confusing commit message, it should have been
"domain_conf:
extend switch in virDomainChrDefFree()" only:D
ok, with that and ...
Kristina
On Thu, Jul 21, 2022 at 12:46 PM Kristina Hanicova <khanicov(a)redhat.com>
wrote:
> Switch is used for just one case, but a more future proof
> approach is to handle all enum values.
>
... without this
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
5:45 a.m.
New subject: [PATCH v2 6/8] domain_conf: use early return in virDomainObjAssignDef()
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 32 +++++++++++++++++---------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 41eb105a6c..507bff953c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3928,22 +3928,24 @@ void virDomainObjAssignDef(virDomainObj *domain,
else
virDomainDefFree(domain->newDef);
domain->newDef = g_steal_pointer(def);
- } else {
- if (live) {
- /* save current configuration to be restored on domain shutdown */
- if (!domain->newDef)
- domain->newDef = domain->def;
- else
- virDomainDefFree(domain->def);
- domain->def = g_steal_pointer(def);
- } else {
- if (oldDef)
- *oldDef = domain->def;
- else
- virDomainDefFree(domain->def);
- domain->def = g_steal_pointer(def);
- }
+ return;
+ }
+
+ if (live) {
+ /* save current configuration to be restored on domain shutdown */
+ if (!domain->newDef)
+ domain->newDef = domain->def;
+ else
+ virDomainDefFree(domain->def);
+ domain->def = g_steal_pointer(def);
+ return;
}
+
+ if (oldDef)
+ *oldDef = domain->def;
+ else
+ virDomainDefFree(domain->def);
+ domain->def = g_steal_pointer(def);
}
--
2.35.3
5:45 a.m.
New subject: [PATCH v2 7/8] domain_conf: rewrite conditions in virDomainObjWaitUntil()
This patch rewrites conditions to make the code easier and less
structured.
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 507bff953c..bbe6574487 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4009,15 +4009,15 @@ int
virDomainObjWaitUntil(virDomainObj *vm,
unsigned long long whenms)
{
- if (virCondWaitUntil(&vm->cond, &vm->parent.lock, whenms) < 0) {
- if (errno != ETIMEDOUT) {
- virReportSystemError(errno, "%s",
- _("failed to wait for domain condition"));
- return -1;
- }
+ if (virCondWaitUntil(&vm->cond, &vm->parent.lock, whenms) >= 0)
+ return 0;
+
+ if (errno == ETIMEDOUT)
return 1;
- }
- return 0;
+
+ virReportSystemError(errno, "%s",
+ _("failed to wait for domain condition"));
+ return -1;
}
--
2.35.3
10:13 a.m.
New subject: [PATCH v2 7/8] domain_conf: rewrite conditions in virDomainObjWaitUntil()
On Thu, Jul 21, 2022 at 12:45:52PM +0200, Kristina Hanicova wrote:
This patch rewrites conditions to make the code easier and less
structured.
"less structured" bears a negative connotation, but I must say the code
is very easy now =D ;)
s/easier/easier to read/; s/structured/nested/
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
5:45 a.m.
New subject: [PATCH v2 8/8] domain_conf: rewrite variable setting
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bbe6574487..d3872830c9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4216,12 +4216,8 @@ virDomainObjGetOneDefState(virDomainObj *vm,
if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
return NULL;
- if (live) {
- if (flags & VIR_DOMAIN_AFFECT_LIVE)
- *live = true;
- else
- *live = false;
- }
+ if (live)
+ *live = flags & VIR_DOMAIN_AFFECT_LIVE;
if (virDomainObjIsActive(vm) && flags & VIR_DOMAIN_AFFECT_CONFIG)
return vm->newDef;
--
2.35.3
10:14 a.m.
New subject: [PATCH v2 8/8] domain_conf: rewrite variable setting
On Thu, Jul 21, 2022 at 12:45:53PM +0200, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/conf/domain_conf.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bbe6574487..d3872830c9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4216,12 +4216,8 @@ virDomainObjGetOneDefState(virDomainObj *vm,
if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
return NULL;
- if (live) {
- if (flags & VIR_DOMAIN_AFFECT_LIVE)
- *live = true;
- else
- *live = false;
- }
+ if (live)
+ *live = flags & VIR_DOMAIN_AFFECT_LIVE;
if (virDomainObjIsActive(vm) && flags & VIR_DOMAIN_AFFECT_CONFIG)
return vm->newDef;
--
2.35.3
890
days inactive
890
days old
17 comments
2 participants
participants (2)
-
Kristina Hanicova -
Martin Kletzander