3:27 a.m.
On 06/06/2013 02:29 PM, Michal Privoznik wrote:
For now, only these three helpers are needed:
virDomainChrFind - to find a duplicate chardev within VM def
virDomainChrInsert - wrapper for inserting a new chardev into VM def
virDomainChrRemove - wrapper for removing chardev from VM def
There is, however, one internal helper as well:
virDomainChrGetDomainPtrs which sets given pointers to one of
vmdef->{parallels,serials,consoles,channels} based on passed
chardev type.
---
src/conf/domain_conf.c | 160 +++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 11 ++++
src/libvirt_private.syms | 4 ++
3 files changed, 175 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a16ebd1..37f0ce9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10056,6 +10056,166 @@ virDomainLeaseRemove(virDomainDefPtr def,
return virDomainLeaseRemoveAt(def, i);
}
+static bool
+virDomainChrEquals(virDomainChrDefPtr src,
+ virDomainChrDefPtr tgt)
+{
+ if (!src || !tgt)
+ return src == tgt;
+
+ if (!virDomainChrSourceDefIsEqual(&src->source, &tgt->source))
+ return false;
+
+ if (src->targetTypeAttr != tgt->targetTypeAttr)
+ return false;
+
+ if (!src->targetTypeAttr)
+ return true;
+
+ switch ((enum virDomainChrChannelTargetType) src->targetType) {
There should be one more check and switch above this one to
differentiate according to src->deviceType and tgt->deviceType.
+ case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
+ return STREQ_NULLABLE(src->target.name, tgt->target.name);
+ break;
+ case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
+ if (!src->target.addr || !tgt->target.addr)
+ return src->target.addr == tgt->target.addr;
+ return memcmp(src->target.addr, tgt->target.addr,
+ sizeof(*src->target.addr)) == 0;
+ break;
+
+ case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_NONE:
+ case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST:
+ default:
No need for default case in here.
+ return true;
+ }
+}
+
+virDomainChrDefPtr
+virDomainChrFind(virDomainDefPtr def,
+ virDomainChrDefPtr target)
+{
+ size_t i;
+
+ for (i = 0; i < def->nparallels; i++) {
+ virDomainChrDefPtr chr = def->parallels[i];
+
+ if (virDomainChrEquals(chr, target))
+ return chr;
+ }
+
+ for (i = 0; i < def->nserials; i++) {
+ virDomainChrDefPtr chr = def->serials[i];
+
+ if (virDomainChrEquals(chr, target))
+ return chr;
+ }
+
+
+ for (i = 0; i < def->nconsoles; i++) {
+ virDomainChrDefPtr chr = def->consoles[i];
+
+ if (virDomainChrEquals(chr, target))
+ return chr;
+ }
+
+ for (i = 0; i < def->nchannels; i++) {
+ virDomainChrDefPtr chr = def->channels[i];
+
+ if (virDomainChrEquals(chr, target))
+ return chr;
+ }
+
You're iterating over all devices in the domain, but you can check which
ones to check based on target->deviceType.
+ return NULL;
+}
+
+int
+virDomainChrGetDomainPtrs(virDomainDefPtr vmdef,
+ virDomainChrDefPtr chr,
+ virDomainChrDefPtr ***arrPtr,
+ size_t **cntPtr)
+{
+ switch ((enum virDomainChrDeviceType) chr->deviceType) {
+ case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL:
+ *arrPtr = &vmdef->parallels;
+ *cntPtr = &vmdef->nparallels;
+ break;
+
+ case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL:
+ *arrPtr = &vmdef->serials;
+ *cntPtr = &vmdef->nserials;
+ break;
+
+ case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
+ *arrPtr = &vmdef->consoles;
+ *cntPtr = &vmdef->nconsoles;
+ break;
+
+ case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL:
+ *arrPtr = &vmdef->channels;
+ *cntPtr = &vmdef->nchannels;
+ break;
+
+ default:
I'd rather cast the switch argument, put a TYPE_LAST here and you don't
have to report the error then.
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("Unsupported device type '%d'"),
+ chr->deviceType);
+ return -1;
+ }
+ return 0;
+}
+
And with this function, you can iterate over those devices in
virDomainChrFind even more clearly.
+int virDomainChrInsert(virDomainDefPtr vmdef,
+ virDomainChrDefPtr chr)
+{
+ virDomainChrDefPtr **arrPtr;
+ size_t *cntPtr;
+
+ if (virDomainChrGetDomainPtrs(vmdef, chr, &arrPtr, &cntPtr) < 0)
+ return -1;
+
+ if (VIR_REALLOC_N(*arrPtr, *cntPtr + 1) < 0) {
+ virReportOOMError();
+ return -1;
+ }
+
+ (*arrPtr)[*cntPtr] = chr;
+ (*cntPtr)++;
+
VIR_APPEND_ELEMENT could make this a bit shorter and readable.
+ return 0;
+}
+
+int virDomainChrRemove(virDomainDefPtr vmdef,
+ virDomainChrDefPtr chr)
+{
+ virDomainChrDefPtr **arrPtr;
+ size_t i, *cntPtr;
+
+ if (virDomainChrGetDomainPtrs(vmdef, chr, &arrPtr, &cntPtr) < 0)
+ return -1;
+
+ for (i = 0; i < *cntPtr; i++) {
+ virDomainChrDefPtr tmp = (*arrPtr)[i];
+
+ if (virDomainChrEquals(tmp, chr))
+ break;
+ }
+
+ if (i == *cntPtr)
+ return -1;
+
This is another part of code you can de-duplicate since it is similar to
the virDomainChrFind (and deals with the problem mentioned there).
+ virDomainChrDefFree((*arrPtr)[i]);
+ if (*cntPtr > 1) {
+ memmove(*arrPtr + i,
+ *arrPtr + i + 1,
+ sizeof(**arrPtr) * (*cntPtr - (i + 1)));
+ ignore_value(VIR_REALLOC_N(*arrPtr, *cntPtr - 1));
+ } else {
+ VIR_FREE(*arrPtr);
+ }
+ (*cntPtr)--;
+
And for this you should be able to use the VIR_DELETE_ELEMENT macro.
+ return 0;
+}
char *
virDomainDefGetDefaultEmulator(virDomainDefPtr def,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3a71d6c..6d650ff 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2355,6 +2355,17 @@ virDomainLeaseDefPtr
virDomainLeaseRemove(virDomainDefPtr def,
virDomainLeaseDefPtr lease);
+int virDomainChrGetDomainPtrs(virDomainDefPtr vmdef,
s/ /\n/
+ virDomainChrDefPtr chr,
+ virDomainChrDefPtr ***arrPtr,
+ size_t **cntPtr);
+virDomainChrDefPtr virDomainChrFind(virDomainDefPtr def,
dtto
+ virDomainChrDefPtr target);
+int virDomainChrInsert(virDomainDefPtr vmdef,
dtto
+ virDomainChrDefPtr chr);
+int virDomainChrRemove(virDomainDefPtr vmdef,
dtto
+ virDomainChrDefPtr chr);
+
int virDomainSaveXML(const char *configDir,
pre-existing, but could be fixed.
virDomainDefPtr def,
const char *xml);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b93629f..b9ba731 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -77,6 +77,10 @@ virDomainChrConsoleTargetTypeToString;
virDomainChrDefForeach;
virDomainChrDefFree;
virDomainChrDefNew;
+virDomainChrFind;
+virDomainChrGetDomainPtrs;
+virDomainChrInsert;
+virDomainChrRemove;
virDomainChrSerialTargetTypeFromString;
virDomainChrSerialTargetTypeToString;
virDomainChrSourceDefCopy;
Martin