On Thu, Feb 13, 2014 at 02:14:32PM +0400, Roman Bogorodskiy wrote:
+static int
+bhyveBuildDiskArgStr(const virDomainDef *def, virCommandPtr cmd)
+{
+ virDomainDiskDefPtr disk;
+
+ if (def->ndisks != 1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("domain should have one and only one disk
defined"));
+ return -1;
+ }
+
+ disk = def->disks[0];
+
+ if (disk->bus != VIR_DOMAIN_DISK_BUS_SATA) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("unsupported disk bus type"));
+ return -1;
+ }
+
+ if (disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("unsupported disk type"));
+ return -1;
+ }
We still need to validate disk->type before accessing disk->src
otherwise you'll crash if someone uses type=network
+
+ virCommandAddArg(cmd, "-s");
+ virCommandAddArgFormat(cmd, "2:0,ahci-hd,%s", disk->src);
What is the '2:0' bit here ? Is that disk controller/unit
number ?
+virCommandPtr
+virBhyveProcessBuildLoadCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED,
+ virDomainObjPtr vm)
+{
+ virCommandPtr cmd;
+ virDomainDiskDefPtr disk;
+
+ if (vm->def->ndisks != 1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("domain should have one and only one disk
defined"));
+ return NULL;
+ }
+
+ disk = vm->def->disks[0];
+
+ if (disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("unsupported disk type"));
+ return NULL;
+ }
And validate disk->type again here
+
+ cmd = virCommandNew(BHYVELOAD);
+
+ /* Memory */
+ virCommandAddArg(cmd, "-m");
+ virCommandAddArgFormat(cmd, "%llu",
+ VIR_DIV_UP(vm->def->mem.max_balloon, 1024));
+
+ /* Image path */
+ virCommandAddArg(cmd, "-d");
+ virCommandAddArgFormat(cmd, disk->src);
+
+ /* VM name */
+ virCommandAddArg(cmd, vm->def->name);
+
+ return cmd;
+}
+static char *
+bhyveConnectGetCapabilities(virConnectPtr conn)
+{
+ bhyveConnPtr privconn = conn->privateData;
+ char *xml;
+
+ if (virConnectGetCapabilitiesEnsureACL(conn) < 0)
+ return NULL;
+
+ bhyveDriverLock(privconn);
+ if ((xml = virCapabilitiesFormatXML(privconn->caps)) == NULL)
+ virReportOOMError();
+ bhyveDriverUnlock(privconn);
+
+ return xml;
+}
Technically the lock/unlock isn't needed since you never
change privconn->caps once you've created it.
+static virDrvOpenStatus
+bhyveConnectOpen(virConnectPtr conn,
+ virConnectAuthPtr auth ATTRIBUTE_UNUSED,
+ unsigned int flags)
+{
+ virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
+
+ if (conn->uri == NULL) {
+ if (bhyve_driver == NULL)
+ return VIR_DRV_OPEN_DECLINED;
+
+ if (!(conn->uri = virURIParse("bhyve:///system")))
+ return VIR_DRV_OPEN_ERROR;
Nitpick indentation is too great in the two 'return' lines
+ } else {
+ if (!conn->uri->scheme || STRNEQ(conn->uri->scheme,
"bhyve"))
+ return VIR_DRV_OPEN_DECLINED;
+
+ if (conn->uri->server)
+ return VIR_DRV_OPEN_DECLINED;
+
+ if (!STREQ_NULLABLE(conn->uri->path, "/system")) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unexpected bhyve URI path '%s', try
bhyve:///system"),
+ conn->uri->path);
+ return VIR_DRV_OPEN_ERROR;
+ }
+
+ if (bhyve_driver == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("bhyve state driver is not
active"));
+ return VIR_DRV_OPEN_ERROR;
+ }
+ }
+
+ if (virConnectOpenEnsureACL(conn) < 0)
+ return VIR_DRV_OPEN_ERROR;
+
+ conn->privateData = bhyve_driver;
+
+ return VIR_DRV_OPEN_SUCCESS;
+}
+static virDomainPtr
+bhyveDomainDefineXML(virConnectPtr conn, const char *xml)
+{
+ bhyveConnPtr privconn = conn->privateData;
+ virDomainPtr dom = NULL;
+ virDomainDefPtr def = NULL;
+ virDomainDefPtr oldDef = NULL;
+ virDomainObjPtr vm = NULL;
+
+ if ((def = virDomainDefParseString(xml, privconn->caps, privconn->xmlopt,
+ 1 << VIR_DOMAIN_VIRT_BHYVE,
+ VIR_DOMAIN_XML_INACTIVE)) == NULL) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Can't parse XML desc"));
Remove the 'virReportError' call, since that's already done for you
by the parsing code.
+ goto cleanup;
+ }
+
+ if (virDomainDefineXMLEnsureACL(conn, def) < 0)
+ goto cleanup;
+
+ if (!(vm = virDomainObjListAdd(privconn->domains, def,
+ privconn->xmlopt,
+ 0, &oldDef)))
+ goto cleanup;
+ def = NULL;
+
+ dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
+ if (dom)
+ dom->id = vm->def->id;
+
+ if (virDomainSaveConfig(BHYVE_CONFIG_DIR, vm->def) < 0)
+ goto cleanup;
+
+cleanup:
+ virDomainDefFree(def);
+ if (vm)
+ virObjectUnlock(vm);
+
+ return dom;
+}
+
+
+int
+virBhyveProcessStart(virConnectPtr conn,
+ bhyveConnPtr driver,
+ virDomainObjPtr vm,
+ virDomainRunningReason reason)
+{
+ char *logfile = NULL;
+ int logfd = -1;
+ off_t pos = -1;
+ char ebuf[1024];
+ virCommandPtr cmd = NULL;
+ virCommandPtr load_cmd = NULL;
+ bhyveConnPtr privconn = conn->privateData;
+ int ret = -1, status;
+
+ if (virAsprintf(&logfile, "%s/%s.log",
+ BHYVE_LOG_DIR, vm->def->name) < 0)
+ return -1;
+
+
+ if ((logfd = open(logfile, O_WRONLY | O_APPEND | O_CREAT,
+ S_IRUSR|S_IWUSR)) < 0) {
+ virReportSystemError(errno,
+ _("Failed to open '%s'"),
+ logfile);
+ goto cleanup;
+ }
+
+ VIR_FREE(privconn->pidfile);
+ if (!(privconn->pidfile = virPidFileBuildPath(BHYVE_STATE_DIR,
+ vm->def->name))) {
+ virReportSystemError(errno,
+ "%s", _("Failed to build pidfile
path."));
+ goto cleanup;
+ }
+
+ if (unlink(privconn->pidfile) < 0 &&
+ errno != ENOENT) {
+ virReportSystemError(errno,
+ _("Cannot remove state PID file %s"),
+ privconn->pidfile);
+ goto cleanup;
+ }
+
+ /* Call bhyve to start the VM */
+ if (!(cmd = virBhyveProcessBuildBhyveCmd(driver,
+ vm)))
+ goto cleanup;
+
+ virCommandSetOutputFD(cmd, &logfd);
+ virCommandSetErrorFD(cmd, &logfd);
+ virCommandWriteArgLog(cmd, logfd);
+ virCommandSetPidFile(cmd, privconn->pidfile);
+ virCommandDaemonize(cmd);
+
+ /* Now bhyve command is constructed, meaning the
+ * domain is ready to be started, so we can build
+ * and execute bhyveload command */
+ if (!(load_cmd = virBhyveProcessBuildLoadCmd(driver,
+ vm)))
+ goto cleanup;
+ virCommandSetOutputFD(load_cmd, &logfd);
+ virCommandSetErrorFD(load_cmd, &logfd);
+
+ /* Log generated command line */
+ virCommandWriteArgLog(load_cmd, logfd);
+ if ((pos = lseek(logfd, 0, SEEK_END)) < 0)
+ VIR_WARN("Unable to seek to end of logfile: %s",
+ virStrerror(errno, ebuf, sizeof(ebuf)));
+
+ VIR_INFO("Loading domain '%s'", vm->def->name);
+ if (virCommandRun(load_cmd, &status) < 0)
+ goto cleanup;
+
+ if (status != 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Guest failed to load: %d"), status);
+ goto cleanup;
+ }
+
+ /* Now we can start the domain */
+ VIR_INFO("Starting domain '%s'", vm->def->name);
I'd suggest s/INFO/DEBUG/ here and earlier. If you want user visible
log messages about key operations, then we should talk about what is
needed to make viraudit.{c,h} work on FreeBSD, and use domain_audit.c
for this.
+ ret = virCommandRun(cmd, NULL);
+
+ if (ret == 0) {
+ if (virPidFileReadPath(privconn->pidfile, &vm->pid) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Domain %s didn't show up"),
vm->def->name);
+ goto cleanup;
+ }
+
+ vm->def->id = vm->pid;
+ virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason);
+ } else {
+ goto cleanup;
+ }
+
+cleanup:
+ if (ret < 0) {
+ virCommandPtr destroy_cmd;
+ if ((destroy_cmd = virBhyveProcessBuildDestroyCmd(driver, vm)) != NULL) {
+ virCommandSetOutputFD(load_cmd, &logfd);
+ virCommandSetErrorFD(load_cmd, &logfd);
+ ignore_value(virCommandRun(destroy_cmd, NULL));
+ virCommandFree(destroy_cmd);
+ }
+ }
+
+ virCommandFree(load_cmd);
+ virCommandFree(cmd);
+ VIR_FREE(logfile);
+ if (VIR_CLOSE(logfd) < 0) {
+ virReportSystemError(errno, "%s", _("could not close
logfile"));
+ }
You can use VIR_FORCE_CLOSE and ignore the error message here.
+ return ret;
+}
Regards,
Daniel
--
|:
http://berrange.com -o-
http://www.flickr.com/photos/dberrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|:
http://entangle-photo.org -o-
http://live.gnome.org/gtk-vnc :|