"Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote:
...
ACK, modulo two questions:
> diff -r a204a9425afd src/domain_conf.c
> int virDomainDeleteConfig(virConnectPtr conn,
> - virDomainObjPtr dom)
> + const char *configDir,
> + const char *autostartDir,
> + virDomainObjPtr dom)
> {
> - if (!dom->configFile || !dom->autostartLink) {
> - virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
> - _("no config file for %s"), dom->def->name);
> - return -1;
> + char *configFile = NULL, *autostartLink = NULL;
> + int ret = -1;
> +
> + if (asprintf(&configFile, "%s/%s",
Shouldn't that be "%s/%s.xml", as used in at least two
other places where configFile is defined?
This deserves a tiny helper function, so that the
dom->configFile mapping happens in just one place.
> + configDir, dom->def->name) < 0) {
> + configFile = NULL;
...
> #endif /* ! PROXY */
...
> diff -r a204a9425afd src/qemu_driver.c
...
> static int qemudDomainSetAutostart(virDomainPtr dom,
> - int autostart) {
> + int autostart) {
> struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
> virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
> + char *configFile = NULL, *autostartLink = NULL;
> + int ret = -1;
>
> if (!vm) {
> qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
> "%s", _("no domain with matching uuid"));
> + return -1;
> + }
> +
> + if (!vm->persistent) {
> + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
> + "%s", _("cannot set autostart for transient domain"));
> return -1;
> }
>
> @@ -3039,6 +3053,20 @@
> if (vm->autostart == autostart)
> return 0;
>
> + if (asprintf(&configFile, "%s/%s.xml",
> + driver->configDir, vm->def->name) < 0) {
> + configFile = NULL;
> + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_MEMORY, NULL);
> + goto cleanup;
> + }
> +
> + if (asprintf(&autostartLink, "%s/%s.xml",
> + driver->autostartDir, vm->def->name) < 0) {
Your 6/7 patch has this:
if (asprintf(&autostartLink, "%s/%s",
Is the different format deliberate?
Maybe a helper function for the dom->autostartLink name, too?
> + autostartLink = NULL;
> + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_MEMORY, NULL);
> + goto cleanup;
> + }
> +
...
> +cleanup:
> + VIR_FREE(configFile);
> + VIR_FREE(autostartLink);
> +
> + return ret;
> }
>
> /* This uses the 'info blockstats' monitor command which was
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list