Fix crash in svirt verification, and incorrect cleanup in VM failure paths

This commit is contained in:
Daniel P. Berrange 2009-04-03 14:10:17 +00:00
parent f0817018b1
commit 9ec1a56923
5 changed files with 89 additions and 58 deletions

View File

@ -1,3 +1,16 @@
Fri Apr 3 15:07:00 BST 2009 Daniel P. Berrange <berrange@redhat.com>
Fix crash in svirt verification, and incorrect cleanup in
VM failure paths.
* src/domain_conf.c: Don't extract 'model' from seclabel unless
requesting 'live' config, or if its a static label. Add missing
error report
* src/qemu_driver.c: Fix cleanup in auto-reconnect to running VMs.
Fix cleanup of resources if starting a new VM fails
* src/security.c: Fix crash if no seclabel model is defined in
the virSecuriyDriverVerify method
* src/security_selinux.c: Fix error message typo & fix whitespace
Fri Apr 3 15:03:00 BST 2009 Daniel P. Berrange <berrange@redhat.com>
* src/virsh.c: Add --console arg for create & start commands

View File

@ -1859,15 +1859,6 @@ virSecurityLabelDefParseXML(virConnectPtr conn,
if (virXPathNode(conn, "./seclabel", ctxt) == NULL)
return 0;
p = virXPathStringLimit(conn, "string(./seclabel/@model)",
VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
if (p == NULL) {
virDomainReportError(conn, VIR_ERR_XML_ERROR,
"%s", _("missing security model"));
goto error;
}
def->seclabel.model = p;
p = virXPathStringLimit(conn, "string(./seclabel/@type)",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p == NULL) {
@ -1888,6 +1879,14 @@ virSecurityLabelDefParseXML(virConnectPtr conn,
*/
if (def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC ||
!(flags & VIR_DOMAIN_XML_INACTIVE)) {
p = virXPathStringLimit(conn, "string(./seclabel/@model)",
VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
if (p == NULL) {
virDomainReportError(conn, VIR_ERR_XML_ERROR,
"%s", _("missing security model"));
goto error;
}
def->seclabel.model = p;
p = virXPathStringLimit(conn, "string(./seclabel/label[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
@ -1905,8 +1904,11 @@ virSecurityLabelDefParseXML(virConnectPtr conn,
!(flags & VIR_DOMAIN_XML_INACTIVE)) {
p = virXPathStringLimit(conn, "string(./seclabel/imagelabel[1])",
VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
if (p == NULL)
if (p == NULL) {
virDomainReportError(conn, VIR_ERR_XML_ERROR,
_("security imagelabel is missing"));
goto error;
}
def->seclabel.imagelabel = p;
}

View File

@ -332,7 +332,7 @@ qemudReconnectVMs(struct qemud_driver *driver)
}
if ((vm->logfile = qemudLogFD(NULL, driver->logDir, vm->def->name)) < 0)
return -1;
goto next_error;
if (vm->def->id >= driver->nextvmid)
driver->nextvmid = vm->def->id + 1;
@ -344,9 +344,12 @@ next_error:
/* we failed to reconnect the vm so remove it's traces */
vm->def->id = -1;
qemudRemoveDomainStatus(NULL, driver, vm);
virDomainDefFree(vm->def);
vm->def = vm->newDef;
vm->newDef = NULL;
/* Restore orignal def, if we'd loaded a live one */
if (vm->newDef) {
virDomainDefFree(vm->def);
vm->def = vm->newDef;
vm->newDef = NULL;
}
next:
virDomainObjUnlock(vm);
if (status)
@ -1319,14 +1322,6 @@ static int qemudStartVMDaemon(virConnectPtr conn,
hookData.vm = vm;
hookData.driver = driver;
/* If you are using a SecurityDriver with dynamic labelling,
then generate a security label for isolation */
if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
driver->securityDriver &&
driver->securityDriver->domainGenSecurityLabel &&
driver->securityDriver->domainGenSecurityLabel(conn, vm) < 0)
return -1;
FD_ZERO(&keepfd);
if (virDomainIsActive(vm)) {
@ -1335,6 +1330,14 @@ static int qemudStartVMDaemon(virConnectPtr conn,
return -1;
}
/* If you are using a SecurityDriver with dynamic labelling,
then generate a security label for isolation */
if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
driver->securityDriver &&
driver->securityDriver->domainGenSecurityLabel &&
driver->securityDriver->domainGenSecurityLabel(conn, vm) < 0)
return -1;
if (vm->def->graphics &&
vm->def->graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
vm->def->graphics->data.vnc.autoport) {
@ -1342,7 +1345,7 @@ static int qemudStartVMDaemon(virConnectPtr conn,
if (port < 0) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"%s", _("Unable to find an unused VNC port"));
return -1;
goto cleanup;
}
vm->def->graphics->data.vnc.port = port;
}
@ -1351,17 +1354,17 @@ static int qemudStartVMDaemon(virConnectPtr conn,
virReportSystemError(conn, errno,
_("cannot create log directory %s"),
driver->logDir);
return -1;
goto cleanup;
}
if((vm->logfile = qemudLogFD(conn, driver->logDir, vm->def->name)) < 0)
return -1;
goto cleanup;
emulator = vm->def->emulator;
if (!emulator)
emulator = virDomainDefDefaultEmulator(conn, vm->def, driver->caps);
if (!emulator)
return -1;
goto cleanup;
/* Make sure the binary we are about to try exec'ing exists.
* Technically we could catch the exec() failure, but that's
@ -1371,7 +1374,7 @@ static int qemudStartVMDaemon(virConnectPtr conn,
virReportSystemError(conn, errno,
_("Cannot find QEMU binary %s"),
emulator);
return -1;
goto cleanup;
}
if (qemudExtractVersionInfo(emulator,
@ -1380,21 +1383,17 @@ static int qemudStartVMDaemon(virConnectPtr conn,
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Cannot determine QEMU argv syntax %s"),
emulator);
return -1;
goto cleanup;
}
if (qemuPrepareHostDevices(conn, vm->def) < 0)
return -1;
goto cleanup;
vm->def->id = driver->nextvmid++;
if (qemudBuildCommandLine(conn, driver, vm,
qemuCmdFlags, &argv, &progenv,
&tapfds, &ntapfds, migrateFrom) < 0) {
close(vm->logfile);
vm->def->id = -1;
vm->logfile = -1;
return -1;
}
&tapfds, &ntapfds, migrateFrom) < 0)
goto cleanup;
tmp = progenv;
while (*tmp) {
@ -1457,12 +1456,8 @@ static int qemudStartVMDaemon(virConnectPtr conn,
"%s", _("Unable to daemonize QEMU process"));
ret = -1;
}
}
if (ret == 0) {
vm->state = migrateFrom ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING;
} else
vm->def->id = -1;
}
for (i = 0 ; argv[i] ; i++)
VIR_FREE(argv[i]);
@ -1479,19 +1474,38 @@ static int qemudStartVMDaemon(virConnectPtr conn,
VIR_FREE(tapfds);
}
if (ret == 0) {
if ((qemudWaitForMonitor(conn, driver, vm, pos) < 0) ||
(qemudDetectVcpuPIDs(conn, vm) < 0) ||
(qemudInitCpus(conn, vm, migrateFrom) < 0) ||
(qemudInitPasswords(conn, driver, vm) < 0) ||
(qemudDomainSetMemoryBalloon(conn, vm, vm->def->memory) < 0) ||
(qemudSaveDomainStatus(conn, qemu_driver, vm) < 0)) {
qemudShutdownVMDaemon(conn, driver, vm);
return -1;
}
if (ret == -1)
goto cleanup;
if ((qemudWaitForMonitor(conn, driver, vm, pos) < 0) ||
(qemudDetectVcpuPIDs(conn, vm) < 0) ||
(qemudInitCpus(conn, vm, migrateFrom) < 0) ||
(qemudInitPasswords(conn, driver, vm) < 0) ||
(qemudDomainSetMemoryBalloon(conn, vm, vm->def->memory) < 0) ||
(qemudSaveDomainStatus(conn, qemu_driver, vm) < 0)) {
qemudShutdownVMDaemon(conn, driver, vm);
ret = -1;
/* No need for 'goto cleanup' now since qemudShutdownVMDaemon does enough */
}
return ret;
cleanup:
if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
VIR_FREE(vm->def->seclabel.model);
VIR_FREE(vm->def->seclabel.label);
VIR_FREE(vm->def->seclabel.imagelabel);
}
if (vm->def->graphics &&
vm->def->graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
vm->def->graphics->data.vnc.autoport)
vm->def->graphics->data.vnc.port = -1;
if (vm->logfile != -1) {
close(vm->logfile);
vm->logfile = -1;
}
vm->def->id = -1;
return -1;
}

View File

@ -33,7 +33,8 @@ virSecurityDriverVerify(virConnectPtr conn, virDomainDefPtr def)
unsigned int i;
const virSecurityLabelDefPtr secdef = &def->seclabel;
if (STREQ(secdef->model, "none"))
if (!secdef->model ||
STREQ(secdef->model, "none"))
return 0;
for (i = 0; security_drivers[i] != NULL ; i++) {
@ -42,7 +43,7 @@ virSecurityDriverVerify(virConnectPtr conn, virDomainDefPtr def)
}
}
virSecurityReportError(conn, VIR_ERR_XML_ERROR,
_("invalid security model"));
_("invalid security model '%s'"), secdef->model);
return -1;
}

View File

@ -162,11 +162,12 @@ SELinuxGenSecurityLabel(virConnectPtr conn,
char *scontext = NULL;
int c1 = 0;
int c2 = 0;
if ( ( vm->def->seclabel.label ) ||
( vm->def->seclabel.model ) ||
( vm->def->seclabel.imagelabel )) {
virSecurityReportError(conn, VIR_ERR_ERROR,
"%s", _("security labellin already defined for VM"));
if (vm->def->seclabel.label ||
vm->def->seclabel.model ||
vm->def->seclabel.imagelabel) {
virSecurityReportError(conn, VIR_ERR_INTERNAL_ERROR,
"%s", _("security label already defined for VM"));
return rc;
}
@ -197,7 +198,7 @@ SELinuxGenSecurityLabel(virConnectPtr conn,
goto err;
}
vm->def->seclabel.model = strdup(SECURITY_SELINUX_NAME);
if (! vm->def->seclabel.model) {
if (!vm->def->seclabel.model) {
virReportOOMError(conn);
goto err;
}