Split qemuDomainChrInsert into two parts

Do the allocation first, then add the actual device.
The second part should never fail. This is good
for live hotplug where we don't want to remove the device
on OOM after the monitor command succeeded.

The only change in behavior is that on failure, the
vmdef->consoles array is freed, not just the first console.

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 10dbabda4465980bf4da954f0066f43a8656b7c9..3c6b77dc6f42cb76fb70e56b8074c73c0500394b 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12027,15 +12027,27 @@ virDomainChrGetDomainPtrs(const virDomainDef *vmdef,
 
 
 int
-virDomainChrInsert(virDomainDefPtr vmdef,
-                   virDomainChrDefPtr chr)
+virDomainChrPreAlloc(virDomainDefPtr vmdef,
+                     virDomainChrDefPtr chr)
+{
+    virDomainChrDefPtr **arrPtr = NULL;
+    size_t *cntPtr = NULL;
+
+    virDomainChrGetDomainPtrsInternal(vmdef, chr->deviceType, &arrPtr, &cntPtr);
+
+    return VIR_REALLOC_N(*arrPtr, *cntPtr + 1);
+}
+
+void
+virDomainChrInsertPreAlloced(virDomainDefPtr vmdef,
+                             virDomainChrDefPtr chr)
 {
     virDomainChrDefPtr **arrPtr = NULL;
     size_t *cntPtr = NULL;
 
     virDomainChrGetDomainPtrsInternal(vmdef, chr->deviceType, &arrPtr, &cntPtr);
 
-    return VIR_APPEND_ELEMENT(*arrPtr, *cntPtr, chr);
+    ignore_value(VIR_APPEND_ELEMENT_INPLACE(*arrPtr, *cntPtr, chr));
 }
 
 virDomainChrDefPtr

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 563c18bdbe3058ffd9d6233a519d8ffde8f966de..93f23143951b08ec08b54d913189844483f887f4 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2652,8 +2652,11 @@ bool
 virDomainChrEquals(virDomainChrDefPtr src,
                    virDomainChrDefPtr tgt);
 int
-virDomainChrInsert(virDomainDefPtr vmdef,
-                   virDomainChrDefPtr chr);
+virDomainChrPreAlloc(virDomainDefPtr vmdef,
+                     virDomainChrDefPtr chr);
+void
+virDomainChrInsertPreAlloced(virDomainDefPtr vmdef,
+                             virDomainChrDefPtr chr);
 virDomainChrDefPtr
 virDomainChrRemove(virDomainDefPtr vmdef,
                    virDomainChrDefPtr chr);

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 75a6d83e3d5e4408ac05de1c31b30e128e710490..bd7870f6a971998258e6388b49da2ff280e3dae3 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -153,7 +153,8 @@ virDomainChrDefNew;
 virDomainChrEquals;
 virDomainChrFind;
 virDomainChrGetDomainPtrs;
-virDomainChrInsert;
+virDomainChrInsertPreAlloced;
+virDomainChrPreAlloc;
 virDomainChrRemove;
 virDomainChrSerialTargetTypeFromString;
 virDomainChrSerialTargetTypeToString;

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1dbcc5ded50939afd0b63dea338904c317deea89..2ea30f5d0409a2b01a0f06db094b9a209ca462c1 100644 (file)
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1403,9 +1403,9 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
 
 }
 
-int
-qemuDomainChrInsert(virDomainDefPtr vmdef,
-                    virDomainChrDefPtr chr)
+static int
+qemuDomainChrPreInsert(virDomainDefPtr vmdef,
+                       virDomainChrDefPtr chr)
 {
     if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
         chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) {
@@ -1420,25 +1420,63 @@ qemuDomainChrInsert(virDomainDefPtr vmdef,
         return -1;
     }
 
-    if (virDomainChrInsert(vmdef, chr) < 0)
+    if (virDomainChrPreAlloc(vmdef, chr) < 0)
         return -1;
 
     /* Due to some crazy backcompat stuff, the first serial device is an alias
      * to the first console too. If this is the case, the definition must be
      * duplicated as first console device. */
-    if (vmdef->nserials == 1 && vmdef->nconsoles == 0) {
-        if ((!vmdef->consoles && VIR_ALLOC(vmdef->consoles) < 0) ||
-            VIR_ALLOC(vmdef->consoles[0]) < 0) {
-            virDomainChrRemove(vmdef, chr);
+    if (vmdef->nserials == 0 && vmdef->nconsoles == 0 &&
+        chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL) {
+        if (!vmdef->consoles && VIR_ALLOC(vmdef->consoles) < 0)
+            return -1;
+
+        if (VIR_ALLOC(vmdef->consoles[0]) < 0) {
+            VIR_FREE(vmdef->consoles);
             return -1;
         }
+        vmdef->nconsoles++;
+    }
+    return 0;
+}
+
+static void
+qemuDomainChrInsertPreAlloced(virDomainDefPtr vmdef,
+                              virDomainChrDefPtr chr)
+{
+    virDomainChrInsertPreAlloced(vmdef, chr);
+    if (vmdef->nserials == 1 && vmdef->nconsoles == 0 &&
+        chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL) {
         vmdef->nconsoles = 1;
 
         /* Create an console alias for the serial port */
         vmdef->consoles[0]->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE;
         vmdef->consoles[0]->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
     }
+}
+
+static void
+qemuDomainChrInsertPreAllocCleanup(virDomainDefPtr vmdef,
+                                   virDomainChrDefPtr chr)
+{
+    /* Remove the stub console added by qemuDomainChrPreInsert */
+    if (vmdef->nserials == 0 && vmdef->nconsoles == 1 &&
+        chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL) {
+        VIR_FREE(vmdef->consoles[0]);
+        VIR_FREE(vmdef->consoles);
+        vmdef->nconsoles = 0;
+    }
+}
 
+int
+qemuDomainChrInsert(virDomainDefPtr vmdef,
+                    virDomainChrDefPtr chr)
+{
+    if (qemuDomainChrPreInsert(vmdef, chr) < 0) {
+        qemuDomainChrInsertPreAllocCleanup(vmdef, chr);
+        return -1;
+    }
+    qemuDomainChrInsertPreAlloced(vmdef, chr);
     return 0;
 }