- Devel - libvirt List Archives

[libvirt] [PATCH 0/2] Detailed XML errors (with thread-local storage this time)
by Richard W.M. Jones 30 Jul '08

30 Jul '08

I've just seen Daniel Veillard's message about the '_private' field. In the meantime here is a solution using thread-local storage implemented on top of pthreads, which may be of interest. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones Read my OCaml programming blog: http://camltastic.blogspot.com/ Fedora now supports 59 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

1 2

[libvirt] PATCH: Validate all test suite XML files against schema
by Daniel P. Berrange 30 Jul '08

30 Jul '08

After frequent prompting from John I've implemented a small test case to validate all the domain XML files we have in the test suite against the defined RNG schema. This showed one minor bug in the schema :-) Daniel diff -r c44225477420 docs/libvirt.rng --- a/docs/libvirt.rng Wed Jul 30 14:20:38 2008 +0100 +++ b/docs/libvirt.rng Wed Jul 30 16:17:19 2008 +0100 @@ -141,6 +141,7 @@ </attribute> <attribute name='machine'> <choice> + <value>xenfv</value> <value>pc</value> <value>isapc</value> </choice> diff -r c44225477420 tests/Makefile.am --- a/tests/Makefile.am Wed Jul 30 14:20:38 2008 +0100 +++ b/tests/Makefile.am Wed Jul 30 16:17:19 2008 +0100 @@ -37,13 +37,14 @@ test-lib.sh \ xmlrpcserver.py \ qemuxml2argvdata \ - nodeinfodata + nodeinfodata \ + domainschematest noinst_PROGRAMS = xmlrpctest xml2sexprtest sexpr2xmltest virshtest conftest \ reconnect xmconfigtest xencapstest qemuxml2argvtest qemuxml2xmltest \ nodeinfotest statstest qparamtest -test_scripts = +test_scripts = domainschematest if WITH_LIBVIRTD test_scripts += \ test_conf.sh \ diff -r c44225477420 tests/domainschematest --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/domainschematest Wed Jul 30 16:17:19 2008 +0100 @@ -0,0 +1,32 @@ +#!/bin/sh + +test -z "$srcdir" && srcdir=`pwd` + +DOMAINDIRS="qemuxml2argvdata sexpr2xmldata xmconfigdata xml2sexprdata" + +n=0 +f=0 +for dir in $DOMAINDIRS +do + XML=`find $dir -name '*.xml'` + + for xml in $XML + do + n=`expr $n + 1` + printf "%4d) %-60s " $n $xml + result=`xmllint --relaxng $srcdir/../docs/libvirt.rng --noout $xml 2>&1` + ret=$? + if test $ret = 0; then + echo "OK" + else + echo "FAILED" + echo $result + f=`expr $f + 1` + fi + done +done +echo "Validated $n files, $f failed" + +ret=0 +test $f != 0 && ret=255 +exit $ret -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1

[libvirt] [PATCH] Detailed XML errors when XML is not well-formed
by Richard W.M. Jones 30 Jul '08

30 Jul '08

This patch changes the implementations lots of functions which parse XML documents, so that if the XML document is not well-formed then we get detailed error messages. The general form of the change is: static void catchXMLError (void *ctx, const char *msg ATTRIBUTE_UNUSED, ...) { // a callback which calls virDomainReportError } virDomainDefPtr virDomainDefParseString(virConnectPtr conn, virCapsPtr caps, const char *xmlStr) { xmlParserCtxtPtr pctxt; pctxt = xmlNewParserCtxt (); pctxt->sax->error = catchXMLError; xml = xmlCtxtReadDoc (pctxt, //...) etc. There are some unavoidable shortcomings: (1) There is no place to stash user pointers during the callback (the suggestively named pctxt->userData field is already used for something else), so we cannot pass the virConnectPtr to the error function. As a result, virterror will store the error in a global variable, and callers will probably not be able to access it. (2) The XML parser routinely produces multiple error messages, and virterror throws away all but the last one. The errors do, however, get printed to stderr. You can test this by using 'virsh define', 'virsh net-define', 'virsh pool-define', etc. on not-well-formed XML files. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v

3 2

[libvirt] libvirt log
by Steve Oliphant 30 Jul '08

30 Jul '08

I have a quick question. I am trying to debug a remote access problem and have set the following parameters: # Override the default config file #LIBVIRTD_CONFIG=/etc/libvirt/libvirtd.conf # Listen for TCP/IP connections # NB. must setup TLS/SSL keys prior to using this LIBVIRTD_ARGS="--listen --verbose" # Override Kerberos service keytab for SASL/GSSAPI #KRB5_KTNAME=/etc/libvirt/krb5.tab I have not been able to find the libvirtd log. Where is it located? Thanks for your help Steve

2 3

[libvirt] [PATCH] virsh edit command
by Richard W.M. Jones 30 Jul '08

30 Jul '08

One thing which is very apparent is that sys admins using libvirt / virsh have a great deal of difficulty understanding "where the configuration files have gone" and how to edit them. This patch adds a "virsh edit <domain>" command which is basically the equivalent of: virsh dumpxml dom > /tmp/dom.xml $EDITOR /tmp/dom.xml && virsh define /tmp/dom.xml but with much more sanity checking. The editor is $EDITOR or vi, and it does the right thing if the user doesn't modify the file, or if another user edits the configuration at the same time. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v

4 19

[libvirt] [PATCH] don't dump core on NULL ifname
by Guido Günther 30 Jul '08

30 Jul '08

Hi, actually I thought I sent this out already, but it seems I didn't: Don't dump core on NULL ifname when getting interface statistic. Not all networking types have a target ifname set (user,client,server,mcast). Cheers, -- Guido

2 1

[libvirt] Small regression with domain refactoring
by Chris Lalancette 30 Jul '08

30 Jul '08

Hey, I've run into another regression when using a KVM guest with libvirt tip. If I have a guest with some XML: <graphics type='vnc' port='-1' listen='127.0.0.1'/> Then when I "virsh define" that guest, it ends up looking like: <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'/> So far, so good. Now, when I actually "virsh start" the guest, it ends up like: <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'/> That is, there is no change to the XML when the domain is running. What is supposed to happen is that the "port" field should be filled in with the port that libvirt ended up choosing for this guest. This is important for tools like virt-viewer, which parse this part of the XML to find out where to connect to. Looking at the code, I believe the problem is in src/domain_conf.c:virDomainGraphicsDefFormat(). It looks like: case VIR_DOMAIN_GRAPHICS_TYPE_VNC: if (def->data.vnc.autoport) virBufferAddLit(buf, " port='-1'"); else if (def->data.vnc.port) virBufferVSprintf(buf, " port='%d'", def->data.vnc.port); The problem is that we are missing some sort of "isActive" check in there, so that it fills in the actual port it is using if the domain is running. Unfortunately, from a quick perusal, I couldn't easily figure out how to do this; does someone have a suggestion on how to fix this? Thanks, Chris Lalancette

2 1

[libvirt] [PATCH] Fix logical storage pool operation on SLES10-SP2
by David Lively 30 Jul '08

30 Jul '08

The attached patch adjusts for a difference in behavior in the LVM utilities 'lvs' and 'vgs'. The SLES10-SP2 versions of these (and presumably others) append a trailing separator. This patch simply adjusts the regexps to allow (but not require) this. I thought just adding the ":?" to the regexps would do this, but this was leaving the trailing separator in the last group match, so I ended up tweaking the preceding group pattern as well. Dave

3 3

[libvirt] [PATCH] qemud: move check for polkit before config file check
by Guido Günther 30 Jul '08

30 Jul '08

Hi, Without this patch and without a /etc/libvirt/libvirt.conf config file the default policy for running the daemon as non root user is still polkit which is bad. Please apply. Cheers, -- Guido

3 2

[Libvir] [PATCH] Add bus attribute to disk target definition
by Soren Hansen 29 Jul '08

29 Jul '08

Hi! I'd like to propose that the following patch gets applied against libvirt. It adds the option of putting a bus attribute on a disk target. To acommodate this, it also changes the way drives are defined for kvm from the old "-hda /path/to/file -boot c" style to the new "-drive file=/path/to/file,if=ide,boot=on". This makes it possible to specify virtio, scsi, and ide disks. === modified file 'src/qemu_conf.c' --- src/qemu_conf.c 2008-04-28 15:14:59 +0000 +++ src/qemu_conf.c 2008-04-29 07:43:11 +0000 @@ -568,6 +568,7 @@ xmlChar *source = NULL; xmlChar *target = NULL; xmlChar *type = NULL; + xmlChar *bus = NULL; int typ = 0; type = xmlGetProp(node, BAD_CAST "type"); @@ -598,6 +599,7 @@ } else if ((target == NULL) && (xmlStrEqual(cur->name, BAD_CAST "target"))) { target = xmlGetProp(cur, BAD_CAST "dev"); + bus = xmlGetProp(cur, BAD_CAST "bus"); } else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) { disk->readonly = 1; } @@ -646,7 +648,9 @@ strcmp((const char *)target, "hda") && strcmp((const char *)target, "hdb") && strcmp((const char *)target, "hdc") && - strcmp((const char *)target, "hdd")) { + strcmp((const char *)target, "hdd") && + strcmp((const char *)target, "hdd") && + strncmp((const char *)target, "vd", 2)) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("Invalid harddisk device name: %s"), target); goto error; @@ -673,6 +677,20 @@ goto error; } + if (!bus) + disk->bus = QEMUD_DISK_BUS_IDE; + else if (!strcmp((const char *)bus, "ide")) + disk->bus = QEMUD_DISK_BUS_IDE; + else if (!strcmp((const char *)bus, "scsi")) + disk->bus = QEMUD_DISK_BUS_SCSI; + else if (!strcmp((const char *)bus, "virtio")) + disk->bus = QEMUD_DISK_BUS_VIRTIO; + else { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "Invalid bus type: %s", bus); + goto error; + } + + xmlFree(bus); xmlFree(device); xmlFree(target); xmlFree(source); @@ -688,6 +706,8 @@ xmlFree(source); if (device) xmlFree(device); + if (bus) + xmlFree(bus); return -1; } @@ -1350,6 +1370,68 @@ return -1; } +static int qemudDiskCompare(const void *aptr, const void *bptr) { + struct qemud_vm_disk_def *a = (struct qemud_vm_disk_def *) aptr; + struct qemud_vm_disk_def *b = (struct qemud_vm_disk_def *) bptr; + if (a->device == b->device) + return virDiskNameToIndex(a->dst) - virDiskNameToIndex(b->dst); + else + return a->device - b->device; +} + +static const char *qemudBusIdToName(int busId) { + const char *busnames[] = { "ide", + "scsi", + "virtio" }; + + if (busId >= 0 && busId < 3) + return busnames[busId]; + else + return 0; +} + +static char *qemudDriveOpt(struct qemud_vm_disk_def *disk, int boot) +{ + char opt[PATH_MAX]; + + switch (disk->device) { + case QEMUD_DISK_CDROM: + snprintf(opt, PATH_MAX, "file=%s,if=ide,media=cdrom%s", + disk->src, boot ? ",boot=on" : ""); + break; + case QEMUD_DISK_FLOPPY: + snprintf(opt, PATH_MAX, "file=%s,if=floppy%s", + disk->src, boot ? ",boot=on" : ""); + break; + case QEMUD_DISK_DISK: + snprintf(opt, PATH_MAX, "file=%s,if=%s%s", + disk->src, qemudBusIdToName(disk->bus), boot ? ",boot=on" : ""); + break; + default: + return 0; + } + return strdup(opt); +} + +static char *qemudAddBootDrive(virConnectPtr conn, + struct qemud_vm_def *def, + char *handledDisks, + int type) { + int j = 0; + struct qemud_vm_disk_def *disk = def->disks; + + while (disk) { + if (!handledDisks[j] && disk->device == type) { + handledDisks[j] = 1; + return qemudDriveOpt(disk, 1); + } + j++; + disk = disk->next; + } + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + "Requested boot device type %d, but no such device defined.", type); + return 0; +} /* * Parses a libvirt XML definition of a guest, and populates the @@ -1739,7 +1821,6 @@ obj = xmlXPathEval(BAD_CAST "/domain/devices/disk", ctxt); if ((obj != NULL) && (obj->type == XPATH_NODESET) && (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr >= 0)) { - struct qemud_vm_disk_def *prev = NULL; for (i = 0; i < obj->nodesetval->nodeNr; i++) { struct qemud_vm_disk_def *disk = calloc(1, sizeof(*disk)); if (!disk) { @@ -1752,13 +1833,20 @@ goto error; } def->ndisks++; - disk->next = NULL; if (i == 0) { + disk->next = NULL; def->disks = disk; } else { - prev->next = disk; + struct qemud_vm_disk_def *ptr = def->disks; + while (ptr) { + if (!ptr->next || qemudDiskCompare(ptr->next, disk) < 0) { + disk->next = ptr->next; + ptr->next = disk; + break; + } + ptr = ptr->next; + } } - prev = disk; } } xmlXPathFreeObject(obj); @@ -2207,30 +2295,32 @@ goto no_memory; } - for (i = 0 ; i < vm->def->os.nBootDevs ; i++) { - switch (vm->def->os.bootDevs[i]) { - case QEMUD_BOOT_CDROM: - boot[i] = 'd'; - break; - case QEMUD_BOOT_FLOPPY: - boot[i] = 'a'; - break; - case QEMUD_BOOT_DISK: - boot[i] = 'c'; - break; - case QEMUD_BOOT_NET: - boot[i] = 'n'; - break; - default: - boot[i] = 'c'; - break; + if (vm->def->virtType != QEMUD_VIRT_KVM) { + for (i = 0 ; i < vm->def->os.nBootDevs ; i++) { + switch (vm->def->os.bootDevs[i]) { + case QEMUD_BOOT_CDROM: + boot[i] = 'd'; + break; + case QEMUD_BOOT_FLOPPY: + boot[i] = 'a'; + break; + case QEMUD_BOOT_DISK: + boot[i] = 'c'; + break; + case QEMUD_BOOT_NET: + boot[i] = 'n'; + break; + default: + boot[i] = 'c'; + break; + } } + boot[vm->def->os.nBootDevs] = '\0'; + if (!((*argv)[++n] = strdup("-boot"))) + goto no_memory; + if (!((*argv)[++n] = strdup(boot))) + goto no_memory; } - boot[vm->def->os.nBootDevs] = '\0'; - if (!((*argv)[++n] = strdup("-boot"))) - goto no_memory; - if (!((*argv)[++n] = strdup(boot))) - goto no_memory; if (vm->def->os.kernel[0]) { if (!((*argv)[++n] = strdup("-kernel"))) @@ -2251,28 +2341,74 @@ goto no_memory; } - while (disk) { - char dev[NAME_MAX]; - char file[PATH_MAX]; - if (!strcmp(disk->dst, "hdc") && - disk->device == QEMUD_DISK_CDROM) { - if (disk->src[0]) - snprintf(dev, NAME_MAX, "-%s", "cdrom"); - else { - /* Don't put anything on the cmdline for an empty cdrom*/ - disk = disk->next; - continue; - } - } else - snprintf(dev, NAME_MAX, "-%s", disk->dst); - snprintf(file, PATH_MAX, "%s", disk->src); - - if (!((*argv)[++n] = strdup(dev))) - goto no_memory; - if (!((*argv)[++n] = strdup(file))) - goto no_memory; - - disk = disk->next; + if (vm->def->virtType == QEMUD_VIRT_KVM) { + char *handledDisks = NULL; + int j; + + handledDisks = calloc(sizeof(*handledDisks), vm->def->ndisks); + + if (!handledDisks) + goto no_memory; + + /* When using -drive notation, we need to provide the devices in boot + * preference order. */ + for (i = 0 ; i < vm->def->os.nBootDevs ; i++) { + if (!((*argv)[++n] = strdup("-drive"))) + goto no_memory; + + switch (vm->def->os.bootDevs[i]) { + case QEMUD_BOOT_CDROM: + if (!((*argv)[++n] = qemudAddBootDrive(conn, vm->def, handledDisks, QEMUD_DISK_CDROM))) + goto error; + break; + case QEMUD_BOOT_FLOPPY: + if (!((*argv)[++n] = qemudAddBootDrive(conn, vm->def, handledDisks, QEMUD_DISK_FLOPPY))) + goto error; + break; + case QEMUD_BOOT_DISK: + if (!((*argv)[++n] = qemudAddBootDrive(conn, vm->def, handledDisks, QEMUD_DISK_DISK))) + goto error; + break; + } + } + + /* Pick up the rest of the devices */ + j=0; + while (disk) { + if (!handledDisks[j]) { + handledDisks[j] = 1; + if (!((*argv)[++n] = strdup("-drive"))) + goto no_memory; + if (!((*argv)[++n] = qemudDriveOpt(disk, 0))) + goto no_memory; + } + disk = disk->next; + j++; + } + } else { + while (disk) { + char dev[NAME_MAX]; + char file[PATH_MAX]; + + if (!strcmp(disk->dst, "hdc") && + disk->device == QEMUD_DISK_CDROM) + if (disk->src[0]) + snprintf(dev, NAME_MAX, "-%s", "cdrom"); + else { + disk = disk->next; + continue; + } + else + snprintf(dev, NAME_MAX, "-%s", disk->dst); + snprintf(file, PATH_MAX, "%s", disk->src); + + if (!((*argv)[++n] = strdup(dev))) + goto no_memory; + if (!((*argv)[++n] = strdup(file))) + goto no_memory; + + disk = disk->next; + } } if (!net) { @@ -3565,6 +3701,7 @@ virBufferVSprintf(&buf, " <source %s='%s'/>\n", typeAttrs[disk->type], disk->src); + virBufferVSprintf(&buf, " <target dev='%s' bus='%s'/>\n", disk->dst, qemudBusIdToName(disk->bus)); virBufferVSprintf(&buf, " <target dev='%s'/>\n", disk->dst); if (disk->readonly) === modified file 'src/qemu_conf.h' --- src/qemu_conf.h 2008-04-25 20:46:13 +0000 +++ src/qemu_conf.h 2008-04-29 07:13:16 +0000 @@ -56,10 +56,17 @@ QEMUD_DISK_FLOPPY, }; +enum qemud_vm_disk_bus { + QEMUD_DISK_BUS_IDE, + QEMUD_DISK_BUS_SCSI, + QEMUD_DISK_BUS_VIRTIO +}; + /* Stores the virtual disk configuration */ struct qemud_vm_disk_def { int type; int device; + int bus; char src[PATH_MAX]; char dst[NAME_MAX]; int readonly; === modified file 'src/util.c' --- src/util.c 2008-04-25 14:53:05 +0000 +++ src/util.c 2008-04-29 06:59:49 +0000 @@ -771,3 +771,43 @@ return -1; } + +/* Translates a device name of the form (regex) "[fhv]d[a-z]+" into + * the corresponding index (e.g. sda => 1, hdz => 26, vdaa => 27) + * @param name The name of the device + * @return name's index, or 0 on failure + */ +int virDiskNameToIndex(const char *name) { + const char *ptr = NULL; + int idx = 0; + + if (strlen(name) < 3) + return 0; + + switch (*name) { + case 'f': + case 'h': + case 'v': + break; + default: + return 0; + } + + if (*(name + 1) != 'd') + return 0; + + ptr = name+2; + + while (*ptr) { + idx = idx * 26; + + if ('a' > *ptr || 'z' < *ptr) + return 0; + + idx += *ptr - 'a' + 1; + ptr++; + } + + return idx; +} + === modified file 'src/util.h' --- src/util.h 2008-04-25 14:53:05 +0000 +++ src/util.h 2008-04-29 06:59:57 +0000 @@ -92,4 +92,6 @@ int virParseMacAddr(const char* str, unsigned char *addr); +int virDiskNameToIndex(const char* str); + #endif /* __VIR_UTIL_H__ */ -- Soren Hansen | Virtualisation specialist | Ubuntu Server Team Canonical Ltd. | http://www.ubuntu.com/

6 17