[Libvir] [patch 0/9] Preliminary patches for virtual networks support
Hi, Yesterday I went ahead and committed the virtual networks support in time for DV to make a release. I thought it'd be worth sending them here so that people can look over them and hopefully sport any problems. So, here's the first batch - various fixes to the qemu support and small bits of re-factoring in other parts of libvirt. Cheers, Mark. --
Invoke autoheader from autogen.sh in order to generate config.h.in. Note, we really should remove config.h.in from CVS. Signed-off-by: Mark McLoughlin <markmc@redhat.com> Index: libvirt-foo/autogen.sh =================================================================== --- libvirt-foo.orig/autogen.sh 2007-02-15 08:39:26.000000000 +0000 +++ libvirt-foo.orig/autogen.sh 2007-02-15 08:39:26.000000000 +0000 @@ -58,6 +58,7 @@ autopoint --force #rm -rf m4 libtoolize --copy --force aclocal -I m4 +autoheader automake --add-missing autoconf --
Include the autoconf generated config.h in all qemud code. Signed-off-by: Mark McLoughlin <markmc@redhat.com> Index: libvirt/qemud/conf.c =================================================================== --- libvirt.orig/qemud/conf.c +++ libvirt/qemud/conf.c @@ -21,6 +21,8 @@ * Author: Daniel P. Berrange <berrange@redhat.com> */ +#include <config.h> + #include <dirent.h> #include <string.h> #include <limits.h> Index: libvirt/qemud/dispatch.c =================================================================== --- libvirt.orig/qemud/dispatch.c +++ libvirt/qemud/dispatch.c @@ -21,6 +21,8 @@ * Author: Daniel P. Berrange <berrange@redhat.com> */ +#include <config.h> + #include <limits.h> #include <string.h> #include <stdlib.h> Index: libvirt/qemud/driver.c =================================================================== --- libvirt.orig/qemud/driver.c +++ libvirt/qemud/driver.c @@ -21,6 +21,8 @@ * Author: Daniel P. Berrange <berrange@redhat.com> */ +#include <config.h> + #include <sys/types.h> #include <sys/poll.h> #include <dirent.h> Index: libvirt/qemud/qemud.c =================================================================== --- libvirt.orig/qemud/qemud.c +++ libvirt/qemud/qemud.c @@ -21,6 +21,8 @@ * Author: Daniel P. Berrange <berrange@redhat.com> */ +#include <config.h> + #include <sys/types.h> #include <sys/wait.h> #include <sys/stat.h> --
Don't share virConnect->handle with the xen hypervisor socket so that we can have a qemud connection open at the same time as a xen hypervisor connection. Signed-off-by: Mark McLoughlin <markmc@redhat.com> Index: libvirt-foo/src/internal.h =================================================================== --- libvirt-foo.orig/src/internal.h 2007-01-23 14:39:45.000000000 +0000 +++ libvirt-foo.orig/src/internal.h 2007-01-23 14:39:45.000000000 +0000 @@ -117,6 +117,8 @@ struct _virConnect { struct sockaddr_un addr_un; /* the unix address */ struct sockaddr_in addr_in; /* the inet address */ + int qemud_fd; /* connection to qemud */ + /* error stuff */ virError err; /* the last error */ virErrorFunc handler; /* associated handlet */ Index: libvirt-foo/src/qemu_internal.c =================================================================== --- libvirt-foo.orig/src/qemu_internal.c 2007-02-14 01:40:09.000000000 +0000 +++ libvirt-foo.orig/src/qemu_internal.c 2007-02-14 01:40:09.000000000 +0000 @@ -244,7 +244,7 @@ qemuOpenClientUNIX(virConnectPtr conn, c return (-1); } - conn->handle = fd; + conn->qemud_fd = fd; return (0); } @@ -269,7 +269,7 @@ static int qemuProcessRequest(virConnect /* Block sending entire outgoing packet */ while (outLeft) { - int got = write(conn->handle, out+outDone, outLeft); + int got = write(conn->qemud_fd, out+outDone, outLeft); if (got < 0) { return -1; } @@ -279,7 +279,7 @@ static int qemuProcessRequest(virConnect /* Block waiting for header to come back */ while (inLeft) { - int done = read(conn->handle, in+inGot, inLeft); + int done = read(conn->qemud_fd, in+inGot, inLeft); if (done <= 0) { return -1; } @@ -307,7 +307,7 @@ static int qemuProcessRequest(virConnect /* Now block reading in body */ inLeft = reply->header.dataSize; while (inLeft) { - int done = read(conn->handle, in+inGot, inLeft); + int done = read(conn->qemud_fd, in+inGot, inLeft); if (done <= 0) { return -1; } @@ -389,11 +389,11 @@ static int qemuOpen(virConnectPtr conn, return -1; } - conn->handle = -1; + conn->qemud_fd = -1; qemuOpenConnection(conn, uri, flags & VIR_DRV_OPEN_RO ? 1 : 0); xmlFreeURI(uri); - if (conn->handle < 0) { + if (conn->qemud_fd < 0) { return -1; } @@ -402,9 +402,9 @@ static int qemuOpen(virConnectPtr conn, static int qemuClose (virConnectPtr conn) { - if (conn->handle != -1) { - close(conn->handle); - conn->handle = -1; + if (conn->qemud_fd != -1) { + close(conn->qemud_fd); + conn->qemud_fd = -1; } return 0; } Index: libvirt-foo/src/hash.c =================================================================== --- libvirt-foo.orig/src/hash.c 2007-01-23 14:39:45.000000000 +0000 +++ libvirt-foo.orig/src/hash.c 2007-01-23 14:39:45.000000000 +0000 @@ -655,6 +655,8 @@ virGetConnect(void) { memset(ret, 0, sizeof(virConnect)); ret->magic = VIR_CONNECT_MAGIC; ret->nb_drivers = 0; + ret->handle = -1; + ret->qemud_fd = -1; ret->domains = virHashCreate(20); if (ret->domains == NULL) goto failed; --
markmc@redhat.com wrote:
Don't share virConnect->handle with the xen hypervisor socket so that we can have a qemud connection open at the same time as a xen hypervisor connection.
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
Index: libvirt-foo/src/internal.h =================================================================== --- libvirt-foo.orig/src/internal.h 2007-01-23 14:39:45.000000000 +0000 +++ libvirt-foo.orig/src/internal.h 2007-01-23 14:39:45.000000000 +0000 @@ -117,6 +117,8 @@ struct _virConnect { struct sockaddr_un addr_un; /* the unix address */ struct sockaddr_in addr_in; /* the inet address */
+ int qemud_fd; /* connection to qemud */ + /* error stuff */ virError err; /* the last error */ virErrorFunc handler; /* associated handlet */ Index: libvirt-foo/src/qemu_internal.c =================================================================== --- libvirt-foo.orig/src/qemu_internal.c 2007-02-14 01:40:09.000000000 +0000 +++ libvirt-foo.orig/src/qemu_internal.c 2007-02-14 01:40:09.000000000 +0000 @@ -244,7 +244,7 @@ qemuOpenClientUNIX(virConnectPtr conn, c return (-1); }
- conn->handle = fd; + conn->qemud_fd = fd;
I don't really understand this patch. Surely if you want to connect to the Xen h/v and qemud you'd use two separate connections? Rich. -- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 "[Negative numbers] darken the very whole doctrines of the equations and make dark of the things which are in their nature excessively obvious and simple" (Francis Maseres FRS, mathematician, 1759)
On Fri, 2007-02-16 at 13:29 +0000, Richard W.M. Jones wrote:
I don't really understand this patch. Surely if you want to connect to the Xen h/v and qemud you'd use two separate connections?
I think it's better if each driver keeps its data separate from the others ... so we should probably have a union or just a pointer to driver private data ... The issue here, though, is that you need the qemud driver for networks support even when connecting to Xen. Cheers, Mark.
Mark McLoughlin wrote:
On Fri, 2007-02-16 at 13:29 +0000, Richard W.M. Jones wrote:
I don't really understand this patch. Surely if you want to connect to the Xen h/v and qemud you'd use two separate connections?
I think it's better if each driver keeps its data separate from the others ... so we should probably have a union or just a pointer to driver private data ...
The issue here, though, is that you need the qemud driver for networks support even when connecting to Xen.
Do you mean the qemu network driver? Rich. -- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 "[Negative numbers] darken the very whole doctrines of the equations and make dark of the things which are in their nature excessively obvious and simple" (Francis Maseres FRS, mathematician, 1759)
Some misc fixes for the qemud network interface code: - don't try and free a NULL macaddr - xmlFree(), unlike all the rest of the xml*Free() functions, isn't NULL safe - handle an unspecifed MAC address (that is allowed, right?) - fix the argv freeing code in qemudBuildCommandLine() - fix copy and paste error in qemudGenerateXML() Signed-off-by: Mark McLoughlin <markmc@redhat.com> Index: libvirt-foo/qemud/conf.c =================================================================== --- libvirt-foo.orig/qemud/conf.c 2007-02-14 14:41:37.000000000 +0000 +++ libvirt-foo.orig/qemud/conf.c 2007-02-14 14:41:37.000000000 +0000 @@ -856,10 +856,16 @@ int qemudBuildCommandLine(struct qemud_s } else { while (net) { char nic[3+1+7+1+17+1]; - sprintf(nic, "nic,macaddr=%02x:%02x:%02x:%02x:%02x:%02x", - net->mac[0], net->mac[1], - net->mac[2], net->mac[3], - net->mac[4], net->mac[5]); + + if (!net->mac[0] && !net->mac[1] && !net->mac[2] && + !net->mac[3] && !net->mac[4] && !net->mac[5]) { + strncpy(nic, "nic", 4); + } else { + sprintf(nic, "nic,macaddr=%02x:%02x:%02x:%02x:%02x:%02x", + net->mac[0], net->mac[1], + net->mac[2], net->mac[3], + net->mac[4], net->mac[5]); + } if (!((*argv)[++n] = strdup("-net"))) goto no_memory; @@ -896,8 +902,8 @@ int qemudBuildCommandLine(struct qemud_s no_memory: if (argv) { for (i = 0 ; i < n ; i++) - free(argv[i]); - free(argv); + free((*argv)[i]); + free(*argv); } qemudReportError(server, VIR_ERR_NO_MEMORY, "argv"); return -1; @@ -1353,8 +1359,7 @@ char *qemudGenerateXML(struct qemud_serv } net = vm->def.nets; - disk = vm->def.disks; - while (disk) { + while (net) { const char *types[] = { "user", "tap", @@ -1367,7 +1372,9 @@ char *qemudGenerateXML(struct qemud_serv types[net->type]) < 0) goto no_memory; - if (qemudBufferPrintf(&buf, " <mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n", + if (net->mac[0] && net->mac[1] && net->mac[2] && + net->mac[3] && net->mac[4] && net->mac[5] && + qemudBufferPrintf(&buf, " <mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n", net->mac[0], net->mac[1], net->mac[2], net->mac[3], net->mac[4], net->mac[5]) < 0) goto no_memory; @@ -1375,7 +1382,7 @@ char *qemudGenerateXML(struct qemud_serv if (qemudBufferPrintf(&buf, " </interface>\n") < 0) goto no_memory; - disk = disk->next; + net = net->next; } if (vm->def.graphicsType == QEMUD_GRAPHICS_VNC) { --
Re-factor bits of conf.c so that: - qemudMakeConfigPath() can be re-used given another configDir - split qemudEnsureConfigDir() out of qemudSaveConfig() so that it may be re-used to create another configDir - split qemudScanConfigDir() out so that qemudScanConfigs() can scan multiple configDirs Note, Dan originally folded this into his qemu patches, but it looks to have since been lost. Signed-off-by: Mark McLoughlin <markmc@redhat.com> Index: libvirt/qemud/conf.c =================================================================== --- libvirt.orig/qemud/conf.c +++ libvirt/qemud/conf.c @@ -93,6 +93,55 @@ static int qemudParseUUID(const char *uu } +/* Build up a fully qualfiied path for a config file to be + * associated with a persistent guest or network */ +static int +qemudMakeConfigPath(const char *configDir, + const char *name, + const char *ext, + char *buf, + unsigned int buflen) { + if ((strlen(configDir) + 1 + strlen(name) + (ext ? strlen(ext) : 0) + 1) > buflen) + return -1; + + strcpy(buf, configDir); + strcat(buf, "/"); + strcat(buf, name); + if (ext) + strcat(buf, ext); + return 0; +} + +static int +qemudEnsureConfigDir(struct qemud_server *server, + const char *configDir) { + struct stat sb; + + /* XXX: needs to be recursive */ + + if (stat(configDir, &sb) < 0) { + if (errno == ENOENT) { + if (mkdir(configDir, 0700) < 0) { + qemudReportError(server, VIR_ERR_INTERNAL_ERROR, + "cannot create config directory %s: %s", + configDir, strerror(errno)); + return -1; + } + } else { + qemudReportError(server, VIR_ERR_INTERNAL_ERROR, + "cannot stat config directory %s", + configDir, strerror(errno)); + return -1; + } + } else if (!S_ISDIR(sb.st_mode)) { + qemudReportError(server, VIR_ERR_INTERNAL_ERROR, + "config directory %s is not a directory", configDir); + return -1; + } + + return 0; +} + struct qemu_arch_info { const char *arch; const char **machines; @@ -929,57 +978,19 @@ void qemudFreeVM(struct qemud_vm *vm) { free(vm); } -/* Build up a fully qualified path for a config file to be - * associated with a persistent guest */ -static -int qemudMakeConfigPath(struct qemud_server *server, - const char *name, - const char *ext, - char *buf, - unsigned int buflen) { - if ((strlen(server->configDir) + 1 + strlen(name) + (ext ? strlen(ext) : 0) + 1) > buflen) - return -1; - - strcpy(buf, server->configDir); - strcat(buf, "/"); - strcat(buf, name); - if (ext) - strcat(buf, ext); - return 0; -} - - /* Save a guest's config data into a persistent file */ static int qemudSaveConfig(struct qemud_server *server, struct qemud_vm *vm) { char *xml; int fd = -1, ret = -1; int towrite; - struct stat sb; if (!(xml = qemudGenerateXML(server, vm))) { return -1; } - if (stat(server->configDir, &sb) < 0) { - if (errno == ENOENT) { - if (mkdir(server->configDir, 0700) < 0) { - qemudReportError(server, VIR_ERR_INTERNAL_ERROR, - "cannot create config directory %s", - server->configDir); - return -1; - } - } else { - qemudReportError(server, VIR_ERR_INTERNAL_ERROR, - "cannot stat config directory %s", - server->configDir); - return -1; - } - } else if (!S_ISDIR(sb.st_mode)) { - qemudReportError(server, VIR_ERR_INTERNAL_ERROR, - "config directory %s is not a directory", - server->configDir); - return -1; + if (qemudEnsureConfigDir(server, server->configDir) < 0) { + goto cleanup; } if ((fd = open(vm->configFile, @@ -1069,7 +1080,7 @@ struct qemud_vm *qemudLoadConfigXML(stru vm->configFile[PATH_MAX-1] = '\0'; } else { if (save) { - if (qemudMakeConfigPath(server, vm->def.name, ".xml", vm->configFile, PATH_MAX) < 0) { + if (qemudMakeConfigPath(server->configDir, vm->def.name, ".xml", vm->configFile, PATH_MAX) < 0) { qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "cannot construct config file path"); qemudFreeVM(vm); @@ -1132,12 +1143,13 @@ static void qemudLoadConfig(struct qemud } -/* Scan for all guest config files */ -int qemudScanConfigs(struct qemud_server *server) { +static +int qemudScanConfigDir(struct qemud_server *server, + const char *configDir) { DIR *dir; struct dirent *entry; - if (!(dir = opendir(server->configDir))) { + if (!(dir = opendir(configDir))) { if (errno == ENOENT) return 0; return -1; @@ -1148,7 +1160,7 @@ int qemudScanConfigs(struct qemud_server if (entry->d_name[0] == '.') continue; - if (qemudMakeConfigPath(server, entry->d_name, NULL, file, PATH_MAX) < 0) + if (qemudMakeConfigPath(configDir, entry->d_name, NULL, file, PATH_MAX) < 0) continue; qemudLoadConfig(server, file); @@ -1159,6 +1171,10 @@ int qemudScanConfigs(struct qemud_server return 0; } +/* Scan for all guest and network config files */ +int qemudScanConfigs(struct qemud_server *server) { + return qemudScanConfigDir(server, server->configDir); +} /* Simple grow-on-demand string buffer */ /* XXX re-factor to shared library */ --
Re-factor out qemudExec() so that it can be used to launch dnsmasq. Note, exec() doesn't take an argc argument, just a null-terminated argv, so there's no need for qemudBuildCommandLine() to return argc. Note, Dan originally folded this into his qemu patches, but it looks to have since been lost. Signed-off-by: Mark McLoughlin <markmc@redhat.com> Index: libvirt/qemud/conf.c =================================================================== --- libvirt.orig/qemud/conf.c +++ libvirt/qemud/conf.c @@ -775,16 +775,15 @@ static int qemudParseXML(struct qemud_se */ int qemudBuildCommandLine(struct qemud_server *server, struct qemud_vm *vm, - char ***argv, - int *argc) { - int n = -1, i; + char ***argv) { + int len, n = -1, i; char memory[50]; char vcpus[50]; char boot[QEMUD_MAX_BOOT_DEVS+1]; struct qemud_vm_disk_def *disk = vm->def.disks; struct qemud_vm_net_def *net = vm->def.nets; - *argc = 1 + /* qemu */ + len = 1 + /* qemu */ 2 + /* machine type */ (vm->def.virtType == QEMUD_VIRT_QEMU ? 1 : 0) + /* Disable kqemu */ 2 * vm->def.ndisks + /* disks*/ @@ -803,7 +802,7 @@ int qemudBuildCommandLine(struct qemud_s sprintf(memory, "%d", vm->def.memory/1024); sprintf(vcpus, "%d", vm->def.vcpus); - if (!(*argv = malloc(sizeof(char *) * (*argc +1)))) + if (!(*argv = malloc(sizeof(char *) * (len+1)))) goto no_memory; if (!((*argv)[++n] = strdup(vm->def.os.binary))) goto no_memory; Index: libvirt/qemud/qemud.c =================================================================== --- libvirt.orig/qemud/qemud.c +++ libvirt/qemud/qemud.c @@ -324,103 +324,110 @@ static int qemudDispatchServer(struct qe } -int qemudStartVMDaemon(struct qemud_server *server, - struct qemud_vm *vm) { - char **argv = NULL; - int argc = 0; - int pid; - int i, ret = -1; - int stdinfd = -1; +static int +qemudExec(struct qemud_server *server, char **argv, + int *retpid, int *outfd, int *errfd) { + int pid, null; int pipeout[2] = {-1,-1}; int pipeerr[2] = {-1,-1}; - if (vm->def.vncPort < 0) - vm->def.vncActivePort = 5900 + server->nextvmid; - else - vm->def.vncActivePort = vm->def.vncPort; - - if (qemudBuildCommandLine(server, vm, &argv, &argc) < 0) - return -1; - - if (1) { /* XXX debug stuff */ - QEMUD_DEBUG("Spawn QEMU '"); - for (i = 0 ; i < argc; i++) { - QEMUD_DEBUG("%s", argv[i]); - if (i == (argc-1)) { - QEMUD_DEBUG("'\n"); - } else { - QEMUD_DEBUG(" "); - } - } - } - - if ((stdinfd = open(_PATH_DEVNULL, O_RDONLY)) < 0) { - qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "cannot open %s", _PATH_DEVNULL); + if ((null = open(_PATH_DEVNULL, O_RDONLY)) < 0) { + qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "cannot open %s : %s", + _PATH_DEVNULL, strerror(errno)); goto cleanup; } - if (pipe(pipeout) < 0) { - qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "cannot create pipe"); - goto cleanup; - } - - if (pipe(pipeerr) < 0) { - qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "cannot create pipe"); + if ((outfd != NULL && pipe(pipeout) < 0) || + (errfd != NULL && pipe(pipeerr) < 0)) { + qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "cannot create pipe : %s", + strerror(errno)); goto cleanup; } if ((pid = fork()) < 0) { - qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "cannot fork child process"); + qemudReportError(server, VIR_ERR_INTERNAL_ERROR, "cannot fork child process : %s", + strerror(errno)); goto cleanup; } if (pid) { /* parent */ - close(stdinfd); - close(pipeout[1]); - close(pipeerr[1]); - qemudSetNonBlock(pipeout[0]); - qemudSetNonBlock(pipeerr[0]); - vm->def.id = server->nextvmid++; - vm->pid = pid; - vm->stdout = pipeout[0]; - vm->stderr = pipeerr[0]; - - } else { /* child */ - int null; - if ((null = open(_PATH_DEVNULL, O_RDONLY)) < 0) - _exit(1); - - if (close(pipeout[0]) < 0) - _exit(1); - if (close(pipeerr[0]) < 0) - _exit(1); - - if (dup2(stdinfd, STDIN_FILENO) < 0) - _exit(1); - if (dup2(pipeout[1], STDOUT_FILENO) < 0) - _exit(1); - if (dup2(pipeerr[1], STDERR_FILENO) < 0) - _exit(1); - - int open_max = sysconf (_SC_OPEN_MAX); - for (i = 0; i < open_max; i++) - if (i != STDOUT_FILENO && - i != STDERR_FILENO && - i != STDIN_FILENO) - close(i); + close(null); + if (outfd) { + close(pipeout[1]); + qemudSetNonBlock(pipeout[0]); + *outfd = pipeout[0]; + } + if (errfd) { + close(pipeerr[1]); + qemudSetNonBlock(pipeerr[0]); + *errfd = pipeerr[0]; + } + *retpid = pid; + return 0; + } - execvp(argv[0], argv); + /* child */ + if (pipeout[0] > 0 && close(pipeout[0]) < 0) + _exit(1); + if (pipeerr[0] > 0 && close(pipeerr[0]) < 0) _exit(1); - } - ret = 0; + if (dup2(null, STDIN_FILENO) < 0) + _exit(1); + if (dup2(pipeout[1] > 0 ? pipeout[1] : null, STDOUT_FILENO) < 0) + _exit(1); + if (dup2(pipeerr[1] > 0 ? pipeerr[1] : null, STDERR_FILENO) < 0) + _exit(1); + + int i, open_max = sysconf (_SC_OPEN_MAX); + for (i = 0; i < open_max; i++) + if (i != STDOUT_FILENO && + i != STDERR_FILENO && + i != STDIN_FILENO) + close(i); + + execvp(argv[0], argv); + + _exit(1); + + return 0; cleanup: + if (pipeerr[0] > 0) + close(pipeerr[0] > 0); + if (pipeerr[1]) + close(pipeerr[1] > 0); + if (pipeout[0]) + close(pipeout[0] > 0); + if (pipeout[1]) + close(pipeout[1] > 0); + if (null > 0) + close(null); + return -1; +} + + +int qemudStartVMDaemon(struct qemud_server *server, + struct qemud_vm *vm) { + char **argv = NULL; + int i, ret = -1; + + if (vm->def.vncPort < 0) + vm->def.vncActivePort = 5900 + server->nextvmid; + else + vm->def.vncActivePort = vm->def.vncPort; + + if (qemudBuildCommandLine(server, vm, &argv) < 0) + return -1; + + if (qemudExec(server, argv, &vm->pid, &vm->stdout, &vm->stderr) == 0) { + vm->def.id = server->nextvmid++; + ret = 0; + } - for (i = 0 ; i < argc ; i++) { + for (i = 0 ; argv[i] ; i++) free(argv[i]); - } free(argv); return ret; --
qemudSaveConfig() will always report a more specific error, so we should avoid overwriting this error. Note, Dan originally folded this into his qemu patches, but it looks to have since been lost. Signed-off-by: Mark McLoughlin <markmc@redhat.com> Index: libvirt/qemud/conf.c =================================================================== --- libvirt.orig/qemud/conf.c +++ libvirt/qemud/conf.c @@ -1087,8 +1087,6 @@ struct qemud_vm *qemudLoadConfigXML(stru } if (qemudSaveConfig(server, vm) < 0) { - qemudReportError(server, VIR_ERR_INTERNAL_ERROR, - "cannot save config file for guest"); qemudFreeVM(vm); return NULL; } --
Re-name virConnect->domains_mux to virConnect->hashes_mux since it will also be used to protect the networks hash. Signed-off-by: Mark McLoughlin <markmc@redhat.com> Index: libvirt/src/hash.c =================================================================== --- libvirt.orig/src/hash.c +++ libvirt/src/hash.c @@ -660,8 +660,8 @@ virGetConnect(void) { ret->domains = virHashCreate(20); if (ret->domains == NULL) goto failed; - ret->domains_mux = xmlNewMutex(); - if (ret->domains_mux == NULL) + ret->hashes_mux = xmlNewMutex(); + if (ret->hashes_mux == NULL) goto failed; ret->uses = 1; @@ -671,8 +671,8 @@ failed: if (ret != NULL) { if (ret->domains != NULL) virHashFree(ret->domains, (virHashDeallocator) virDomainFreeName); - if (ret->domains_mux != NULL) - xmlFreeMutex(ret->domains_mux); + if (ret->hashes_mux != NULL) + xmlFreeMutex(ret->hashes_mux); free(ret); } return(NULL); @@ -691,22 +691,22 @@ int virFreeConnect(virConnectPtr conn) { int ret; - if ((!VIR_IS_CONNECT(conn)) || (conn->domains_mux == NULL)) { + if ((!VIR_IS_CONNECT(conn)) || (conn->hashes_mux == NULL)) { virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); return(-1); } - xmlMutexLock(conn->domains_mux); + xmlMutexLock(conn->hashes_mux); conn->uses--; ret = conn->uses; if (ret > 0) { - xmlMutexUnlock(conn->domains_mux); + xmlMutexUnlock(conn->hashes_mux); return(ret); } if (conn->domains != NULL) virHashFree(conn->domains, (virHashDeallocator) virDomainFreeName); - if (conn->domains_mux != NULL) - xmlFreeMutex(conn->domains_mux); + if (conn->hashes_mux != NULL) + xmlFreeMutex(conn->hashes_mux); free(conn); return(0); } @@ -729,11 +729,11 @@ virGetDomain(virConnectPtr conn, const c virDomainPtr ret = NULL; if ((!VIR_IS_CONNECT(conn)) || ((name == NULL) && (uuid == NULL)) || - (conn->domains_mux == NULL)) { + (conn->hashes_mux == NULL)) { virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); return(NULL); } - xmlMutexLock(conn->domains_mux); + xmlMutexLock(conn->hashes_mux); /* TODO search by UUID first as they are better differenciators */ @@ -771,11 +771,11 @@ virGetDomain(virConnectPtr conn, const c conn->uses++; done: ret->uses++; - xmlMutexUnlock(conn->domains_mux); + xmlMutexUnlock(conn->hashes_mux); return(ret); error: - xmlMutexUnlock(conn->domains_mux); + xmlMutexUnlock(conn->hashes_mux); if (ret != NULL) { if (ret->name != NULL) free(ret->name ); @@ -799,11 +799,11 @@ virFreeDomain(virConnectPtr conn, virDom int ret = 0; if ((!VIR_IS_CONNECT(conn)) || (!VIR_IS_CONNECTED_DOMAIN(domain)) || - (domain->conn != conn) || (conn->domains_mux == NULL)) { + (domain->conn != conn) || (conn->hashes_mux == NULL)) { virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); return(-1); } - xmlMutexLock(conn->domains_mux); + xmlMutexLock(conn->hashes_mux); /* * decrement the count for the domain @@ -839,13 +839,13 @@ virFreeDomain(virConnectPtr conn, virDom if (conn->domains != NULL) virHashFree(conn->domains, (virHashDeallocator) virDomainFreeName); - if (conn->domains_mux != NULL) - xmlFreeMutex(conn->domains_mux); + if (conn->hashes_mux != NULL) + xmlFreeMutex(conn->hashes_mux); free(conn); return(0); done: - xmlMutexUnlock(conn->domains_mux); + xmlMutexUnlock(conn->hashes_mux); return(ret); } @@ -870,7 +870,7 @@ virGetDomainByID(virConnectPtr conn, int virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); return(NULL); } - xmlMutexLock(conn->domains_mux); + xmlMutexLock(conn->hashes_mux); table = conn->domains; if ((table == NULL) || (table->nbElems == 0)) @@ -890,7 +890,7 @@ virGetDomainByID(virConnectPtr conn, int } } done: - xmlMutexUnlock(conn->domains_mux); + xmlMutexUnlock(conn->hashes_mux); return(ret); } /* Index: libvirt/src/internal.h =================================================================== --- libvirt.orig/src/internal.h +++ libvirt/src/internal.h @@ -125,7 +125,7 @@ struct _virConnect { void *userData; /* the user data */ /* misc */ - xmlMutexPtr domains_mux;/* a mutex to protect the domain hash table */ + xmlMutexPtr hashes_mux;/* a mutex to protect the domain hash table */ virHashTablePtr domains;/* hash table for known domains */ int flags; /* a set of connection flags */ }; --
Re-name some of the VSH_DOMBYFOO stuff to VSH_BYFOO in order to re-use it for the network stuff. Signed-off-by: Mark McLoughlin <markmc@redhat.com> Index: libvirt-foo/src/virsh.c =================================================================== --- libvirt-foo.orig/src/virsh.c 2007-02-14 14:29:40.000000000 +0000 +++ libvirt-foo.orig/src/virsh.c 2007-02-14 14:29:40.000000000 +0000 @@ -10,7 +10,7 @@ * Daniel P. Berrange <berrange@redhat.com> * * - * $Id: virsh.c,v 1.49 2007/02/14 01:40:09 berrange Exp $ + * $Id: virsh.c,v 1.50 2007/02/14 16:04:55 markmc Exp $ */ #define _GNU_SOURCE /* isblank() */ @@ -194,9 +194,9 @@ static char *vshCommandOptString(vshCmd int *found); static int vshCommandOptBool(vshCmd * cmd, const char *name); -#define VSH_DOMBYID (1 << 1) -#define VSH_DOMBYUUID (1 << 2) -#define VSH_DOMBYNAME (1 << 3) +#define VSH_BYID (1 << 1) +#define VSH_BYUUID (1 << 2) +#define VSH_BYNAME (1 << 3) static virDomainPtr vshCommandOptDomainBy(vshControl * ctl, vshCmd * cmd, const char *optname, char **name, int flag); @@ -204,7 +204,7 @@ static virDomainPtr vshCommandOptDomainB /* default is lookup by Id, Name and UUID */ #define vshCommandOptDomain(_ctl, _cmd, _optname, _name) \ vshCommandOptDomainBy(_ctl, _cmd, _optname, _name, \ - VSH_DOMBYID|VSH_DOMBYUUID|VSH_DOMBYNAME) + VSH_BYID|VSH_BYUUID|VSH_BYNAME) static void vshPrintExtra(vshControl * ctl, const char *format, ...); static void vshDebug(vshControl * ctl, int level, const char *format, ...); @@ -226,6 +226,25 @@ static void *_vshCalloc(vshControl * ctl static char *_vshStrdup(vshControl * ctl, const char *s, const char *filename, int line); #define vshStrdup(_ctl, _s) _vshStrdup(_ctl, _s, __FILE__, __LINE__) + +static int idsorter(const void *a, const void *b) { + const int *ia = (const int *)a; + const int *ib = (const int *)b; + + if (*ia > *ib) + return 1; + else if (*ia < *ib) + return -1; + return 0; +} +static int namesorter(const void *a, const void *b) { + const char **sa = (const char**)a; + const char **sb = (const char**)b; + + return strcasecmp(*sa, *sb); +} + + /* --------------- * Commands * --------------- @@ -392,22 +411,6 @@ static vshCmdOptDef opts_list[] = { }; -static int domidsorter(const void *a, const void *b) { - const int *ia = (const int *)a; - const int *ib = (const int *)b; - - if (*ia > *ib) - return 1; - else if (*ia < *ib) - return -1; - return 0; -} -static int domnamesorter(const void *a, const void *b) { - const char **sa = (const char**)a; - const char **sb = (const char**)b; - - return strcasecmp(*sa, *sb); -} static int cmdList(vshControl * ctl, vshCmd * cmd ATTRIBUTE_UNUSED) { @@ -437,7 +440,7 @@ cmdList(vshControl * ctl, vshCmd * cmd A return FALSE; } - qsort(&ids[0], maxid, sizeof(int), domidsorter); + qsort(&ids[0], maxid, sizeof(int), idsorter); } } if (inactive) { @@ -459,7 +462,7 @@ cmdList(vshControl * ctl, vshCmd * cmd A return FALSE; } - qsort(&names[0], maxname, sizeof(char*), domnamesorter); + qsort(&names[0], maxname, sizeof(char*), namesorter); } } vshPrintExtra(ctl, "%3s %-20s %s\n", _("Id"), _("Name"), _("State")); @@ -1544,7 +1547,7 @@ cmdDomname(vshControl * ctl, vshCmd * cm if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; if (!(dom = vshCommandOptDomainBy(ctl, cmd, "domain", NULL, - VSH_DOMBYID|VSH_DOMBYUUID))) + VSH_BYID|VSH_BYUUID))) return FALSE; vshPrint(ctl, "%s\n", virDomainGetName(dom)); @@ -1575,7 +1578,7 @@ cmdDomid(vshControl * ctl, vshCmd * cmd) if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; if (!(dom = vshCommandOptDomainBy(ctl, cmd, "domain", NULL, - VSH_DOMBYNAME|VSH_DOMBYUUID))) + VSH_BYNAME|VSH_BYUUID))) return FALSE; id = virDomainGetID(dom); @@ -1610,7 +1613,7 @@ cmdDomuuid(vshControl * ctl, vshCmd * cm if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; if (!(dom = vshCommandOptDomainBy(ctl, cmd, "domain", NULL, - VSH_DOMBYNAME|VSH_DOMBYID))) + VSH_BYNAME|VSH_BYID))) return FALSE; if (virDomainGetUUIDString(dom, uuid) != -1) @@ -2088,7 +2091,7 @@ vshCommandOptDomainBy(vshControl * ctl, *name = n; /* try it by ID */ - if (flag & VSH_DOMBYID) { + if (flag & VSH_BYID) { id = (int) strtol(n, &end, 10); if (id >= 0 && end && *end == '\0') { vshDebug(ctl, 5, "%s: <%s> seems like domain ID\n", @@ -2097,13 +2100,13 @@ vshCommandOptDomainBy(vshControl * ctl, } } /* try it by UUID */ - if (dom==NULL && (flag & VSH_DOMBYUUID) && strlen(n)==VIR_UUID_STRING_BUFLEN-1) { + if (dom==NULL && (flag & VSH_BYUUID) && strlen(n)==VIR_UUID_STRING_BUFLEN-1) { vshDebug(ctl, 5, "%s: <%s> tring as domain UUID\n", cmd->def->name, optname); dom = virDomainLookupByUUIDString(ctl->conn, n); } /* try it by NAME */ - if (dom==NULL && (flag & VSH_DOMBYNAME)) { + if (dom==NULL && (flag & VSH_BYNAME)) { vshDebug(ctl, 5, "%s: <%s> tring as domain NAME\n", cmd->def->name, optname); dom = virDomainLookupByName(ctl->conn, n); --
On 2/15/07, markmc@redhat.com <markmc@redhat.com> wrote:
Hi, Yesterday I went ahead and committed the virtual networks support in time for DV to make a release. I thought it'd be worth sending them here so that people can look over them and hopefully sport any problems.
I had to rush the release a bit since I'm leaving for vacations, sorry for the pressure, we probably have some bugs in 0.2.0, but now that it is out it will be easier to spot than just by reading patches. Among the things we need to do at this point is code cleanup, add a lib subdirectory with shared code like UUID parsing, XML/Xpath convenience routines, and maybe some network operations which are likely to be common to the daemons. I will try to keep an eye, but I won't be very active in the next 2-3 weeks :-) Daniel
participants (4)
-
Daniel Veillard -
Mark McLoughlin -
markmc＠redhat.com
-
Richard W.M. Jones