September 2012 - Devel - libvirt List Archives

[libvirt] [PATCH] security: also parse user/group names instead of just IDs for DAC labels
by Marcelo Cerri 20 Sep '12

20 Sep '12

The DAC driver is missing parsing of group and user names for DAC labels and currently just parses uid and gid. This patch extends it to support names, so the following security label definition is now valid: <seclabel type='static' model='dac' relabel='yes'> <label>qemu:qemu</label> <imagelabel>qemu:qemu</imagelabel> </seclabel> --- src/security/security_dac.c | 49 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index be65d6e..7e11e31 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -66,28 +66,59 @@ void virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr, } static +int parseId(const char *str, unsigned int *id) +{ + char *endptr = NULL; + + if (str == NULL || id == NULL) + return -1; + + if (virStrToLong_ui(str, &endptr, 10, id) || endptr != NULL) + return -1; + + return 0; +} + +static int parseIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) { + int rc = -1; unsigned int theuid; unsigned int thegid; - char *endptr = NULL; + char *sep = NULL; + char *tmp_label = NULL; if (label == NULL) - return -1; + goto done; - if (virStrToLong_ui(label, &endptr, 10, &theuid) || - endptr == NULL || *endptr != ':') { - return -1; - } + tmp_label = strdup(label); + if (tmp_label == NULL) + goto done; - if (virStrToLong_ui(endptr + 1, NULL, 10, &thegid)) - return -1; + sep = strchr(tmp_label, ':'); + if (sep == NULL) + goto done; + *sep = '\0'; + + if (virGetUserID(tmp_label, &theuid) < 0 && + parseId(tmp_label, &theuid) < 0) + goto done; + + if (virGetGroupID(sep + 1, &thegid) < 0 && + parseId(sep + 1, &thegid) < 0) + goto done; if (uidPtr) *uidPtr = theuid; if (gidPtr) *gidPtr = thegid; - return 0; + + rc = 0; + +done: + VIR_FREE(tmp_label); + + return rc; } /* returns 1 if label isn't found, 0 on success, -1 on error */ -- 1.7.12

4 9

[libvirt] [PATCH v2 0/3] Support for limiting guest coredumps
by Martin Kletzander 20 Sep '12

20 Sep '12

Even though I got a sparse ACK on v1, it's been some time, all the things should be fixed. Also the QEMU patch is already upstream. This series applies on the disable_s3/s4 series [1] and works with qemu patch [2] (upstream already). Basically qemu supports marking guest memory as (not)dumpable using madvise(2) system call and in case the guest memory is not desired the option can reduce the coredump by a reasonable size. [1] https://www.redhat.com/archives/libvir-list/2012-August/msg00572.html [2] http://lists.nongnu.org/archive/html/qemu-devel/2012-08/msg00554.html Martin Kletzander (3): Add support for limiting guest coredump qemu: add support for dump-guest-core option tests: Add tests for dump-core option docs/formatdomain.html.in | 12 +++- docs/schemas/domaincommon.rng | 8 +++ src/conf/domain_conf.c | 25 ++++++- src/conf/domain_conf.h | 10 +++ src/libvirt_private.syms | 2 + src/qemu/qemu_capabilities.c | 4 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 81 +++++++++++++++++++--- tests/qemuargv2xmltest.c | 2 + tests/qemuhelptest.c | 3 +- .../qemuxml2argv-machine-core-off.args | 5 ++ .../qemuxml2argv-machine-core-off.xml | 26 +++++++ .../qemuxml2argv-machine-core-on.args | 5 ++ .../qemuxml2argv-machine-core-on.xml | 26 +++++++ tests/qemuxml2argvtest.c | 3 + tests/qemuxml2xmltest.c | 2 + 16 files changed, 201 insertions(+), 14 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-core-off.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-core-off.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-core-on.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-core-on.xml -- 1.7.12

3 13

[libvirt] [PATCH v2 0/3] Add support for reboot-timeout
by Martin Kletzander 20 Sep '12

20 Sep '12

This series introduces simple reboot timeout support. That means what should be done in case all boot options fail, reboot or not. And if yes, then how long the machine should wait. Before adding the support for that into XML parser, builder, and qemu, I felt the need to cleanup the boot parsing and enums (I couldn't look at that), so that's the first patch. Most of it is mechanical, the rest should be pretty straight-forward. --- v2: - modified according to mprivozn and danpb - ACK'd patch not sent Martin Kletzander (3): Add support for reboot-timeout qemu: Add support for reboot-timeout QEMU Tests for reboot-timeout docs/formatdomain.html.in | 11 +++++-- docs/schemas/domaincommon.rng | 24 +++++++++++---- src/conf/domain_conf.c | 34 ++++++++++++++++++---- src/conf/domain_conf.h | 3 ++ src/qemu/qemu_capabilities.c | 4 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 28 ++++++++++++++++++ tests/qemuargv2xmltest.c | 4 +++ .../qemuxml2argv-reboot-timeout-disabled.args | 3 ++ .../qemuxml2argv-reboot-timeout-disabled.xml | 21 +++++++++++++ .../qemuxml2argv-reboot-timeout-enabled.args | 3 ++ .../qemuxml2argv-reboot-timeout-enabled.xml | 21 +++++++++++++ tests/qemuxml2argvtest.c | 5 ++++ tests/qemuxml2xmltest.c | 4 +++ 14 files changed, 151 insertions(+), 15 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-reboot-timeout-disabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-reboot-timeout-disabled.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-reboot-timeout-enabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-reboot-timeout-enabled.xml -- 1.7.12

2 11

[libvirt] [PATCHv3] security: Don't ignore errors when parsing DAC security labels
by Peter Krempa 20 Sep '12

20 Sep '12

The DAC security driver silently ignored errors when parsing the DAC label and used default values instead. With a domain containing the following label definition: <seclabel type='static' model='dac' relabel='yes'> <label>sdfklsdjlfjklsdjkl</label> </seclabel> the domain would start normaly but the disk images would be still owned by root and no error was displayed. This patch changes the behavior if the parsing of the label fails (note that a not present label is not a failure and in this case the default label should be used) the error isn't masked but is raised that causes the domain start to fail with a descriptive error message: virsh # start tr error: Failed to start domain tr error: internal error invalid argument: failed to parse DAC label 'sdfklsdjlfjklsdjkl' for domain 'tr' I also changed the error code to "invalid argument" from "internal error" and tweaked the various error messages to contain correct and useful information. --- Diff to v2: - Fixed all error reporting paths to contain useful messages - Tweaked error messages to contain more information - Fixed printing of seclabel->label instead of seclabel->imagelabel in the imagelabel parsing func. src/security/security_dac.c | 95 +++++++++++++++++++++++++++++---------------- 1 file changed, 61 insertions(+), 34 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 211fb37..be65d6e 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -90,6 +90,7 @@ int parseIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) return 0; } +/* returns 1 if label isn't found, 0 on success, -1 on error */ static int virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr) { @@ -98,20 +99,18 @@ int virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr) virSecurityLabelDefPtr seclabel; if (def == NULL) - return -1; + return 1; seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); if (seclabel == NULL || seclabel->label == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("security label for DAC not found in domain %s"), - def->name); - return -1; + VIR_DEBUG("DAC seclabel for domain '%s' wasn't found", def->name); + return 1; } if (seclabel->label && parseIds(seclabel->label, &uid, &gid)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to parse uid and gid for DAC " - "security driver: %s"), seclabel->label); + virReportError(VIR_ERR_INVALID_ARG, + _("failed to parse DAC label '%s' for domain '%s'"), + seclabel->label, def->name); return -1; } @@ -127,19 +126,35 @@ static int virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv, uid_t *uidPtr, gid_t *gidPtr) { - if (virSecurityDACParseIds(def, uidPtr, gidPtr) == 0) - return 0; + int ret; - if (priv) { - if (uidPtr) - *uidPtr = priv->user; - if (gidPtr) - *gidPtr = priv->group; - return 0; + if (!def && !priv) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to determine default DAC label " + "for an unknown object")); + return -1; } - return -1; + + if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) <= 0) + return ret; + + if (!priv) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("DAC security label couldn't be determined " + "for domain '%s'"), def->name); + return -1; + } + + if (uidPtr) + *uidPtr = priv->user; + if (gidPtr) + *gidPtr = priv->group; + + return 0; } + +/* returns 1 if label isn't found, 0 on success, -1 on error */ static int virSecurityDACParseImageIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr) @@ -149,21 +164,19 @@ int virSecurityDACParseImageIds(virDomainDefPtr def, virSecurityLabelDefPtr seclabel; if (def == NULL) - return -1; + return 1; seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); if (seclabel == NULL || seclabel->imagelabel == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("security label for DAC not found in domain %s"), - def->name); - return -1; + VIR_DEBUG("DAC imagelabel for domain '%s' wasn't found", def->name); + return 1; } if (seclabel->imagelabel && parseIds(seclabel->imagelabel, &uid, &gid)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to parse uid and gid for DAC " - "security driver: %s"), seclabel->label); + virReportError(VIR_ERR_INVALID_ARG, + _("failed to parse DAC imagelabel '%s' for domain '%s'"), + seclabel->imagelabel, def->name); return -1; } @@ -179,17 +192,31 @@ static int virSecurityDACGetImageIds(virDomainDefPtr def, virSecurityDACDataPtr priv, uid_t *uidPtr, gid_t *gidPtr) { - if (virSecurityDACParseImageIds(def, uidPtr, gidPtr) == 0) - return 0; + int ret; - if (priv) { - if (uidPtr) - *uidPtr = priv->user; - if (gidPtr) - *gidPtr = priv->group; - return 0; + if (!def && !priv) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to determine default DAC imagelabel " + "for an unknown object")); + return -1; + } + + if ((ret = virSecurityDACParseImageIds(def, uidPtr, gidPtr)) <= 0) + return ret; + + if (!priv) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("DAC security imagelabel couldn't be determined " + "for domain '%s'"), def->name); + return -1; } - return -1; + + if (uidPtr) + *uidPtr = priv->user; + if (gidPtr) + *gidPtr = priv->group; + + return 0; } -- 1.7.12

2 2

[libvirt] [PATCH WIP] build: add a warning when configure is executed with -Werror
by Marcelo Cerri 20 Sep '12

20 Sep '12

Some tests performed by configure to check what is supported can be broken if -Werror is defined in CFLAGS variable when configure is executed. This patch checks if CFLAGS contains -Werror and shows a warning for user recommending to use --enable-compile-warnings=error instead. --- Please, note that this is a work in progress patch and suggestions are welcome. configure.ac | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/configure.ac b/configure.ac index 3e90672..38d6263 100644 --- a/configure.ac +++ b/configure.ac @@ -3232,3 +3232,10 @@ AC_MSG_NOTICE([Privileges]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([ QEMU: $QEMU_USER:$QEMU_GROUP]) AC_MSG_NOTICE([]) + +# -Werror can break some configure tests +if echo "$CFLAGS" | sed -e 's/\s\+/\n/g' | grep -E '^-Werror$' >/dev/null 2>&1; then + AC_MSG_WARN([CFLAGS contains -Werror flag. This might break some tests performed by]) + AC_MSG_WARN([configure and cause some compilation errors.]) + AC_MSG_WARN([Please use --enable-compile-warnings=error instead.]) +fi -- 1.7.12

3 2

[libvirt] [PATCH] virsh: new net-update command
by Laine Stump 20 Sep '12

20 Sep '12

This new virsh command uses the new virNetworkUpdate() API to modify an existing network definition, and optionally have those modifications take effect immediately without restarting the network. An example usage: virsh net-update add-last ip-dhcp-host \ --xml "<host mac='00:11:22:33:44:55' ip='192.168.122.45'/>" \ --live --config If you like, you can instead put the xml into a file, and call like this: virsh net-update add-last ip-dhcp-host \ --file "<host mac='00:11:22:33:44:55' ip='192.168.122.45'/>" \ --live --config (or just leave out the "--file", since that is the default). --live, --config, and --current options - if you specify --live, only the live state of the network will be updated. If you also specify --config, then the persistent configuration will also be updated; these two commands can be given separately, or both together. If you don't specify either (you can optionally specify "--current" for the same effect), then the "current" config will be updated (i.e. if the network is active, then only its live config is affected, but if the network is inactive, only the persistent config is affected). A --parent-index option is also available (to give the index within a list of parent objects, e.g. the index of the parent <ip> element when updating ip-dhcp-host elements), but is optional and at least for now will probably be used rarely. --- tools/virsh-network.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 2c32a78..64be260 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -737,6 +737,163 @@ cmdNetworkUndefine(vshControl *ctl, const vshCmd *cmd) } /* + * "net-update" command + */ +static const vshCmdInfo info_network_update[] = { + {"help", N_("update parts of an existing network's configuration")}, + {"desc", ""}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_network_update[] = { + {"network", VSH_OT_DATA, VSH_OFLAG_REQ, N_("network name or uuid")}, + {"command", VSH_OT_DATA, VSH_OFLAG_REQ, + N_("type of update (add, delete, or modify)")}, + {"section", VSH_OT_DATA, VSH_OFLAG_REQ, + N_("which section of network configuration to update")}, + {"xml", VSH_OT_BOOL, 0, N_("xml is specified directly on commandline")}, + {"file", VSH_OT_BOOL, 0, N_("file containing xml is specified on commandline")}, + {"xmldata", VSH_OT_DATA, VSH_OFLAG_REQ, + N_("complete xml element (or name of file containing xml) to add/modify, " + "or to be matched for search")}, + {"parent-index", VSH_OT_INT, 0, N_("which parent object to search through")}, + {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, + {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, + {"current", VSH_OT_BOOL, 0, N_("affect current domain")}, + {NULL, 0, 0, NULL} +}; + +VIR_ENUM_DECL(virNetworkUpdateCommand) +VIR_ENUM_IMPL(virNetworkUpdateCommand, VIR_NETWORK_UPDATE_COMMAND_LAST, + "none", "modify", "delete", "add-last", "add-first"); + +VIR_ENUM_DECL(virNetworkSection) +VIR_ENUM_IMPL(virNetworkSection, VIR_NETWORK_SECTION_LAST, + "none", "bridge", "domain", "ip", "ip-dhcp-host", + "ip-dhcp-range", "forward", "forward-interface", + "forward-pf", "portgroup", "dns-host", "dns-txt", + "dns-srv"); + +static bool +cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; + virNetworkPtr network; + const char *commandStr = NULL; + const char *sectionStr = NULL; + int command, section, parentIndex = -1; + const char *xmldata = NULL; + char *xmldataFromFile = NULL; + bool isFile = vshCommandOptBool(cmd, "file"); + bool isXml = vshCommandOptBool(cmd, "xml"); + bool current = vshCommandOptBool(cmd, "current"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + unsigned int flags = 0; + const char *affected; + + if (!(network = vshCommandOptNetwork(ctl, cmd, NULL))) + goto cleanup; + + if (vshCommandOptString(cmd, "command", &commandStr) < 0) { + vshError(ctl, "%s", _("missing or malformed command argument")); + goto cleanup; + } + command = virNetworkUpdateCommandTypeFromString(commandStr); + if (command <= 0 || command >= VIR_NETWORK_UPDATE_COMMAND_LAST) { + vshError(ctl, _("unrecognized command name '%s'"), commandStr); + goto cleanup; + } + + if (vshCommandOptString(cmd, "section", &sectionStr) < 0) { + vshError(ctl, "%s", _("missing or malformed section argument")); + goto cleanup; + } + section = virNetworkSectionTypeFromString(sectionStr); + if (section <= 0 || section >= VIR_NETWORK_SECTION_LAST) { + vshError(ctl, _("unrecognized section name '%s'"), sectionStr); + goto cleanup; + } + + if (vshCommandOptInt(cmd, "parent-index", &parentIndex) < 0) { + vshError(ctl, "%s", _("malformed parent-index argument")); + goto cleanup; + } + + /* the goal is to have a full xml element in the "xmldata" + * string. This can either be directly given with the --xml + * option, or indirectly with the --file option (default, + * indicates that the xmldata on the commandline is really the + * name of a file whose contents are the desired xml). + */ + + if (!isXml) + isFile = true; + if (isXml && isFile) { + vshError(ctl, "%s", _("you can specify file (default) or xml, " + "but not both")); + goto cleanup; + } + if (vshCommandOptString(cmd, "xmldata", (const char **)&xmldata) < 0) { + vshError(ctl, "%s", _("malformed xmldata argument")); + goto cleanup; + } + + if (isFile) { + /* contents of xmldata is actually the name of a file that + * contains the xml. + */ + if (virFileReadAll(xmldata, VSH_MAX_XML_FILE, &xmldataFromFile) < 0) + return false; + /* NB: the original xmldata is freed by the vshCommand + * infrastructure, so it's safe to lose its pointer here. + */ + xmldata = xmldataFromFile; + } + + if (current) { + if (live || config) { + vshError(ctl, "%s", _("--current must be specified exclusively")); + return false; + } + flags |= VIR_NETWORK_UPDATE_AFFECT_CURRENT; + } else { + if (config) + flags |= VIR_NETWORK_UPDATE_AFFECT_CONFIG; + if (live) + flags |= VIR_NETWORK_UPDATE_AFFECT_LIVE; + } + + if (virNetworkUpdate(network, command, + section, parentIndex, xmldata, flags) < 0) { + vshError(ctl, _("Failed to update network %s"), + virNetworkGetName(network)); + goto cleanup; + } + + if (flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) { + if (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE) + affected = _("persistent config and live state"); + else + affected = _("persistent config"); + } else if (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE) { + affected = _("live state"); + } else if (virNetworkIsActive(network)) { + affected = _("live state"); + } else { + affected = _("persistent config"); + } + + vshPrint(ctl, _("Updated network %s %s"), + virNetworkGetName(network), affected); + ret = true; +cleanup: + virNetworkFree(network); + VIR_FREE(xmldataFromFile); + return ret; +} + +/* * "net-uuid" command */ static const vshCmdInfo info_network_uuid[] = { @@ -854,6 +1011,7 @@ const vshCmdDef networkCmds[] = { {"net-start", cmdNetworkStart, opts_network_start, info_network_start, 0}, {"net-undefine", cmdNetworkUndefine, opts_network_undefine, info_network_undefine, 0}, + {"net-update", cmdNetworkUpdate, opts_network_update, info_network_update, 0}, {"net-uuid", cmdNetworkUuid, opts_network_uuid, info_network_uuid, 0}, {NULL, NULL, NULL, NULL, 0} }; -- 1.7.11.4

3 5

[libvirt] [PATCH] qemu: Transition domain to PAUSED after 'stop' command
by Michal Privoznik 20 Sep '12

20 Sep '12

Currently, we mark domain PAUSED (but not emit an event) just before we issue 'stop' on monitor; This command can take ages to finish, esp. when domain's doing a lot of IO - users can enforce qemu to open files with O_DIRECT which doesn't return from write() until data reaches the block device. Having said that, we report PAUSED even if domain is not paused yet. --- The event is emitted correctly after all operations returns though. But if mgmt app would 'virsh domstate $dom' as we are issuing 'stop' monitor command it could get spurious results. src/qemu/qemu_process.c | 7 +------ 1 files changed, 1 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c28f5a5..b6eb342 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2708,13 +2708,9 @@ int qemuProcessStopCPUs(struct qemud_driver *driver, virDomainObjPtr vm, enum qemuDomainAsyncJob asyncJob) { int ret; - int oldState; - int oldReason; qemuDomainObjPrivatePtr priv = vm->privateData; VIR_FREE(priv->lockState); - oldState = virDomainObjGetState(vm, &oldReason); - virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, reason); ret = qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob); if (ret == 0) { @@ -2723,11 +2719,10 @@ int qemuProcessStopCPUs(struct qemud_driver *driver, virDomainObjPtr vm, } if (ret == 0) { + virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, reason); if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) VIR_WARN("Unable to release lease on %s", vm->def->name); VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState)); - } else { - virDomainObjSetState(vm, oldState, oldReason); } return ret; -- 1.7.8.6

2 3

Re: [libvirt] [PATCH] block: Set cdrom device read only flag
by Kevin Shanahan 20 Sep '12

20 Sep '12

On Thu, Aug 02, 2012 at 11:46:13AM +0930, Kevin Shanahan wrote: > On Thu, Aug 02, 2012 at 11:02:42AM +0930, Kevin Shanahan wrote: > > Set the block driver read_only flag for cdrom devices so that > > qmp_change_blockdev does not attempt to open cdrom files in read-write > > mode when changing media. > > Hrm, this fixes my simple test case using the kvm monitor directly but > changing media via libvirt still has the same issue (fails for RO > files, succeeds for writable files). > > $ virsh attach-disk --type cdrom --mode readonly test1 /srv/kvm/iso/ubuntu-12.04-server-amd64.iso hdc > error: Failed to attach disk > error: internal error unable to execute QEMU command 'change': Could not open '/srv/kvm/iso/ubuntu-12.04-server-amd64.iso' > > I'll keep looking into it. In the libvirt case, it seems libvirt is failing to add media=cdrom to the commandline, so in this case qemu is defaulting to media=disk and my proposed fix has no effect. Diving into libvirt now to see why no media=disk is getting added... Common test case has this xml (generated by virt-install): <disk type='block' device='cdrom'> <driver name='qemu' type='raw'/> <target dev='hdc' bus='ide'/> <readonly/> <alias name='ide0-1-0'/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> > Cheers, > Kevin.

4 11

[libvirt] [PATCH v2] Fix libvirtd crash possibility
by Martin Kletzander 20 Sep '12

20 Sep '12

When generating RPC protocol messages, it's strictly needed to have continuousline of numbers or RPC messages. However in case anyone tries backporting some functionality and will skip a number, there is a possibility to make the daemon segfault with newer virsh (version of the library, rpc call, etc.) even unintentionally. The problem is that the skipped numbers will get func filled with NULLs, but there is no check whether these are set before the daemon tries to run them. This patch very simply enhances one check and fixes that. --- src/rpc/virnetserverprogram.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c index d13b621..95a6d76 100644 --- a/src/rpc/virnetserverprogram.c +++ b/src/rpc/virnetserverprogram.c @@ -1,7 +1,7 @@ /* * virnetserverprogram.c: generic network RPC server program * - * Copyright (C) 2006-2011 Red Hat, Inc. + * Copyright (C) 2006-2012 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -109,12 +109,19 @@ int virNetServerProgramMatches(virNetServerProgramPtr prog, static virNetServerProgramProcPtr virNetServerProgramGetProc(virNetServerProgramPtr prog, int procedure) { + virNetServerProgramProcPtr proc; + if (procedure < 0) return NULL; if (procedure >= prog->nprocs) return NULL; - return &prog->procs[procedure]; + proc = &prog->procs[procedure]; + + if (!proc->func) + return NULL; + + return proc; } unsigned int -- 1.7.12

2 5

[libvirt] [PATCH] network: fix element size in memmove
by Laine Stump 20 Sep '12

20 Sep '12

The memmove to move elements in the dhcp hosts array when inserting and deleting items was mistakenly basing the length of the copy on the size of a virNetworkDHCPHostDefPtr rather than virNetworkDHCPHostDef, with the expected disastrous results. --- We really should create a VIR_SOMETHING() macro to take care of this... src/conf/network_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index db398ae..046891c 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2449,7 +2449,7 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDefPtr def, } else { /* implied (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) */ memmove(ipdef->hosts + 1, ipdef->hosts, - sizeof(ipdef->hosts) * ipdef->nhosts); + sizeof(*ipdef->hosts) * ipdef->nhosts); ipdef->hosts[0] = host; ipdef->nhosts++; memset(&host, 0, sizeof(host)); @@ -2481,7 +2481,7 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDefPtr def, /* remove it */ virNetworkDHCPHostDefClear(&ipdef->hosts[ii]); memmove(ipdef->hosts + ii, ipdef->hosts + ii + 1, - sizeof(ipdef->hosts) * ipdef->nhosts - ii - 1); + sizeof(*ipdef->hosts) * ipdef->nhosts - ii - 1); ipdef->nhosts--; ignore_value(VIR_REALLOC_N(ipdef->hosts, ipdef->nhosts)); } -- 1.7.11.4

2 1