Re: [libvirt] [PATCH v2 03/15] vbox: Cleanup partially-defined VM on failure

Since the VBOX API requires to register an initial VM before proceeding to attach any remaining devices to it, any failure to attach such devices should result in automatic cleanup of the initially registered VM so that the state of VBOX registry remains clean without any leftover "aborted" VMs in it. Failure to cleanup of such partial VMs results in a warning log so that actual define error stays on the top of the error stack. --- src/vbox/vbox_common.c | 41 +++++++++++++++++++++++++++------------- - 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 92ee37164..812c940e6 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -1853,6 +1853,8 @@ vboxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainPtr ret = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; + bool machineReady = false; + virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); @@ -1862,12 +1864,12 @@ vboxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags if (!data->vboxObj) return ret; - VBOX_IID_INITIALIZE(&mchiid); if (!(def = virDomainDefParseString(xml, data->caps, data- >xmlopt, NULL, parse_flags))) { - goto cleanup; + return ret; } + VBOX_IID_INITIALIZE(&mchiid); virUUIDFormat(def->uuid, uuidstr); rc = gVBoxAPI.UIVirtualBox.CreateMachine(data, def, &machine, uuidstr); @@ -1959,30 +1961,41 @@ vboxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags vboxAttachUSB(def, data, machine); vboxAttachSharedFolder(def, data, machine); - /* Save the machine settings made till now and close the - * session. also free up the mchiid variable used. + machineReady = true; + + cleanup: + /* Save the machine settings made till now, even when jumped here on error, + * as otherwise unregister won't cleanup properly. For example, it won't + * close media that were partially attached. The VBOX SDK docs say that + * unregister implicitly calls saveSettings but evidently it's not so... */ rc = gVBoxAPI.UIMachine.SaveSettings(machine);