[libvirt] [PATCH 00/12] Temporarily use other boot configuration

This patch series implements a new API that allows us to temporarily use another boot configuration than defined in the persistent domain definition. The s390 architecture knows only one boot device and therefore the boot order settings doesn't work the way it would work on x86, for example. If the first boot device fails to boot there is no trying to boot from the next boot device. In addition, the architecture/BIOS has no support for interactively changing the boot device during the boot/IPL process. Currently the API is implemented for the remote/QEMU/test driver and for virsh. It can be used as follows $ virsh start {{DOMAIN}} --with-kernel {{KERNEL_FILE}} \ --with-initrd {{INITRD_FILE}} --with-cmdline {{CMDLINE}} \ --with-bootdevice {{DEVICE_IDENTIFIER}} E.g. $ virsh start dom_01 --with-bootdevice='vdb' Marc Hartmayer (12): virsh: Force boot emulation is only required for virDomainCreateWithFlags Introduce new domain create API virDomainCreateWithParams remote: Add support for virDomainCreateWithParams utils: Add virStringUpdate conf: Add functions to change the boot configuration of a domain qemu: Add the functionality to override the boot configuration qemu: Add support for virDomainCreateWithParams test: Implement virConnectSupportsFeature test: Add support for virDomainCreateWithParams tests: Add tests for virDomainCreateWithParams virsh: Add with-{bootdevice,kernel,initrd,cmdline} options for start docs: Add news entry for new API virDomainCreateWithParams docs/news.xml | 11 ++ include/libvirt/libvirt-domain.h | 37 +++++ src/conf/domain_conf.c | 226 +++++++++++++++++++++++++++ src/conf/domain_conf.h | 11 ++ src/driver-hypervisor.h | 6 + src/libvirt-domain.c | 64 ++++++++ src/libvirt_private.syms | 2 + src/libvirt_public.syms | 4 + src/qemu/qemu_driver.c | 83 ++++++++-- src/qemu/qemu_migration.c | 3 +- src/qemu/qemu_process.c | 16 +- src/qemu/qemu_process.h | 2 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 22 ++- src/remote_protocol-structs | 12 ++ src/rpc/gendispatch.pl | 18 ++- src/test/test_driver.c | 108 +++++++++++-- src/util/virstring.c | 27 ++++ src/util/virstring.h | 2 + tests/objecteventtest.c | 321 +++++++++++++++++++++++++++++++++++++++ tools/virsh-domain.c | 136 ++++++++++++++--- tools/virsh.pod | 14 ++ 22 files changed, 1064 insertions(+), 62 deletions(-) -- 2.13.4

The force boot emulation is only required for virDomainCreateWithFlags as the flag VIR_DOMAIN_START_FORCE_BOOT was introduced with the commit 27c85260532f879be5674a4eed0811c21fd34f94 (2011). But virDomainCreateXMLWithFiles is newer and therefore already had support for VIR_DOMAIN_START_FORCE_BOOT. This means there is now no second call with VIR_DOMAIN_START_FORCE_BOOT removed from flags for virDomainCreateXMLWithFiles in case the first call fails. virDomainCreateXMLWithFiles was introduced with commit d76227bea35cc49374a94414f6d46e3493ac2a52 (2013). This patch takes this into account and simplifies the function. In addition, it's now easier to extend the function. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- tools/virsh-domain.c | 52 ++++++++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 598d2fa4a4bd..7cf8373f05bc 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4038,40 +4038,44 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "force-boot")) flags |= VIR_DOMAIN_START_FORCE_BOOT; - /* We can emulate force boot, even for older servers that reject it. */ - if (flags & VIR_DOMAIN_START_FORCE_BOOT) { - if ((nfds ? - virDomainCreateWithFiles(dom, nfds, fds, flags) : - virDomainCreateWithFlags(dom, flags)) == 0) - goto started; - if (last_error->code != VIR_ERR_NO_SUPPORT && - last_error->code != VIR_ERR_INVALID_ARG) { - vshReportError(ctl); - goto cleanup; - } - vshResetLibvirtError(); - rc = virDomainHasManagedSaveImage(dom, 0); - if (rc < 0) { - /* No managed save image to remove */ - vshResetLibvirtError(); - } else if (rc > 0) { - if (virDomainManagedSaveRemove(dom, 0) < 0) { + /* Prefer older API unless we have to pass extra parameters */ + if (nfds) { + rc = virDomainCreateWithFiles(dom, nfds, fds, flags); + } else if (flags) { + rc = virDomainCreateWithFlags(dom, flags); + /* We can emulate force boot, even for older servers that + * reject it. */ + if (rc < 0 && flags & VIR_DOMAIN_START_FORCE_BOOT) { + if (last_error->code != VIR_ERR_NO_SUPPORT && + last_error->code != VIR_ERR_INVALID_ARG) { vshReportError(ctl); goto cleanup; } + vshResetLibvirtError(); + rc = virDomainHasManagedSaveImage(dom, 0); + if (rc < 0) { + /* No managed save image to remove */ + vshResetLibvirtError(); + } else if (rc > 0) { + if (virDomainManagedSaveRemove(dom, 0) < 0) { + vshReportError(ctl); + goto cleanup; + } + } + + /* now try it again without the force boot flag */ + flags &= ~VIR_DOMAIN_START_FORCE_BOOT; + rc = virDomainCreateWithFlags(dom, flags); } - flags &= ~VIR_DOMAIN_START_FORCE_BOOT; + } else { + rc = virDomainCreate(dom); } - /* Prefer older API unless we have to pass a flag. */ - if ((nfds ? virDomainCreateWithFiles(dom, nfds, fds, flags) : - (flags ? virDomainCreateWithFlags(dom, flags) - : virDomainCreate(dom))) < 0) { + if (rc < 0) { vshError(ctl, _("Failed to start domain %s"), virDomainGetName(dom)); goto cleanup; } - started: vshPrintExtra(ctl, _("Domain %s started\n"), virDomainGetName(dom)); #ifndef WIN32 -- 2.13.4

On Wed, May 09, 2018 at 04:56 PM +0200, Marc Hartmayer <mhartmay@linux.vnet.ibm.com> wrote:
The force boot emulation is only required for virDomainCreateWithFlags as the flag VIR_DOMAIN_START_FORCE_BOOT was introduced with the commit 27c85260532f879be5674a4eed0811c21fd34f94 (2011). But virDomainCreateXMLWithFiles is newer and therefore already had support for VIR_DOMAIN_START_FORCE_BOOT. This means there is now no second call with VIR_DOMAIN_START_FORCE_BOOT removed from flags for virDomainCreateXMLWithFiles in case the first call fails. virDomainCreateXMLWithFiles was introduced with commit d76227bea35cc49374a94414f6d46e3493ac2a52 (2013).
This patch takes this into account and simplifies the function. In addition, it's now easier to extend the function.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- tools/virsh-domain.c | 52 ++++++++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 24 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 598d2fa4a4bd..7cf8373f05bc 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4038,40 +4038,44 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "force-boot")) flags |= VIR_DOMAIN_START_FORCE_BOOT;
- /* We can emulate force boot, even for older servers that reject it. */ - if (flags & VIR_DOMAIN_START_FORCE_BOOT) { - if ((nfds ? - virDomainCreateWithFiles(dom, nfds, fds, flags) : - virDomainCreateWithFlags(dom, flags)) == 0) - goto started; - if (last_error->code != VIR_ERR_NO_SUPPORT && - last_error->code != VIR_ERR_INVALID_ARG) { - vshReportError(ctl); - goto cleanup; - } - vshResetLibvirtError(); - rc = virDomainHasManagedSaveImage(dom, 0); - if (rc < 0) { - /* No managed save image to remove */ - vshResetLibvirtError(); - } else if (rc > 0) { - if (virDomainManagedSaveRemove(dom, 0) < 0) { + /* Prefer older API unless we have to pass extra parameters */ + if (nfds) { + rc = virDomainCreateWithFiles(dom, nfds, fds, flags); + } else if (flags) { + rc = virDomainCreateWithFlags(dom, flags); + /* We can emulate force boot, even for older servers that + * reject it. */ + if (rc < 0 && flags & VIR_DOMAIN_START_FORCE_BOOT) { + if (last_error->code != VIR_ERR_NO_SUPPORT && + last_error->code != VIR_ERR_INVALID_ARG) { vshReportError(ctl); goto cleanup; } + vshResetLibvirtError(); + rc = virDomainHasManagedSaveImage(dom, 0); + if (rc < 0) { + /* No managed save image to remove */ + vshResetLibvirtError(); + } else if (rc > 0) { + if (virDomainManagedSaveRemove(dom, 0) < 0) { + vshReportError(ctl); + goto cleanup; + } + } + + /* now try it again without the force boot flag */ + flags &= ~VIR_DOMAIN_START_FORCE_BOOT; + rc = virDomainCreateWithFlags(dom, flags); } - flags &= ~VIR_DOMAIN_START_FORCE_BOOT; + } else { + rc = virDomainCreate(dom); }
- /* Prefer older API unless we have to pass a flag. */ - if ((nfds ? virDomainCreateWithFiles(dom, nfds, fds, flags) : - (flags ? virDomainCreateWithFlags(dom, flags) - : virDomainCreate(dom))) < 0) { + if (rc < 0) { vshError(ctl, _("Failed to start domain %s"), virDomainGetName(dom)); goto cleanup; }
- started: vshPrintExtra(ctl, _("Domain %s started\n"), virDomainGetName(dom)); #ifndef WIN32 -- 2.13.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Since this patch is not directly related to the proposed API it may be still useful => polite ping :) Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On 05/09/2018 10:56 AM, Marc Hartmayer wrote:
The force boot emulation is only required for virDomainCreateWithFlags as the flag VIR_DOMAIN_START_FORCE_BOOT was introduced with the commit 27c85260532f879be5674a4eed0811c21fd34f94 (2011). But virDomainCreateXMLWithFiles is newer and therefore already had support for VIR_DOMAIN_START_FORCE_BOOT. This means there is now no second call with VIR_DOMAIN_START_FORCE_BOOT removed from flags for virDomainCreateXMLWithFiles in case the first call fails. virDomainCreateXMLWithFiles was introduced with commit d76227bea35cc49374a94414f6d46e3493ac2a52 (2013).
This patch takes this into account and simplifies the function. In addition, it's now easier to extend the function.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- tools/virsh-domain.c | 52 ++++++++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 24 deletions(-)
OK so I took the bait ;-)... I agree 110% what you've changed to is easier to read; however, I think there's a subtle difference with your changes... I'm not sure this new flow will work quite right "if" for some reason someone passes the --force-boot flag to/for a startup that uses CreateWithFiles, such as lxc.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 598d2fa4a4bd..7cf8373f05bc 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4038,40 +4038,44 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "force-boot")) flags |= VIR_DOMAIN_START_FORCE_BOOT;> - /* We can emulate force boot, even for older servers that reject it. */
FWIW: I see that this hunk was added by commit id 691ec08b shortly after adding support for force_boot via commit id 27c85260.
- if (flags & VIR_DOMAIN_START_FORCE_BOOT) { - if ((nfds ? - virDomainCreateWithFiles(dom, nfds, fds, flags) : - virDomainCreateWithFlags(dom, flags)) == 0) - goto started; - if (last_error->code != VIR_ERR_NO_SUPPORT && - last_error->code != VIR_ERR_INVALID_ARG) { - vshReportError(ctl); - goto cleanup; - }
Previously if flags & VIR_DOMAIN_START_FORCE_BOOT, we'd try w/ CreateWithFiles without changing @flags. For lxc, eventually we'd end up in lxcDomainCreateWithFiles which would do a virCheckFlags and "fail" with VIR_ERR_INVALID_ARG because the flag VIR_DOMAIN_START_FORCE_BOOT is not supported. That would fall through this removed code and remove the FORCE_BOOT flag. Then we'd again call virDomainCreateWithFiles w/ @flags adjusted to remove VIR_DOMAIN_START_FORCE_BOOT
- vshResetLibvirtError(); - rc = virDomainHasManagedSaveImage(dom, 0); - if (rc < 0) { - /* No managed save image to remove */ - vshResetLibvirtError(); - } else if (rc > 0) { - if (virDomainManagedSaveRemove(dom, 0) < 0) { + /* Prefer older API unless we have to pass extra parameters */ + if (nfds) { + rc = virDomainCreateWithFiles(dom, nfds, fds, flags);
With this new code if @flags has VIR_DOMAIN_START_FORCE_BOOT, then there's no fallback and we fail.
+ } else if (flags) { + rc = virDomainCreateWithFlags(dom, flags); + /* We can emulate force boot, even for older servers that + * reject it. */ + if (rc < 0 && flags & VIR_DOMAIN_START_FORCE_BOOT) { + if (last_error->code != VIR_ERR_NO_SUPPORT && + last_error->code != VIR_ERR_INVALID_ARG) {
The CreateWithFlags code could "fail" w/ INVALID_ARG if some hypervisor didn't support some flag, but even calling it again without the FORCE_BOOT may not change the result. This (existing, I know) code assumes the failure is from the lack of a FORCE_BOOT flag as added via commit 691ec08b. Having commit id afb50d79 further complicate the logic especially w/r/t FORCE_BOOT (which I think makes no sense for a container hypervisor, but then again I'm not the expert there ;-)). Still since it's in the API and documented, it's not like we could remove it (whether or not it was an unintentional cut-copy-paste, change API name). John
vshReportError(ctl); goto cleanup; } + vshResetLibvirtError(); + rc = virDomainHasManagedSaveImage(dom, 0); + if (rc < 0) { + /* No managed save image to remove */ + vshResetLibvirtError(); + } else if (rc > 0) { + if (virDomainManagedSaveRemove(dom, 0) < 0) { + vshReportError(ctl); + goto cleanup; + } + } + + /* now try it again without the force boot flag */ + flags &= ~VIR_DOMAIN_START_FORCE_BOOT; + rc = virDomainCreateWithFlags(dom, flags); } - flags &= ~VIR_DOMAIN_START_FORCE_BOOT; + } else { + rc = virDomainCreate(dom); }
- /* Prefer older API unless we have to pass a flag. */ - if ((nfds ? virDomainCreateWithFiles(dom, nfds, fds, flags) : - (flags ? virDomainCreateWithFlags(dom, flags) - : virDomainCreate(dom))) < 0) { + if (rc < 0) { vshError(ctl, _("Failed to start domain %s"), virDomainGetName(dom)); goto cleanup; }
- started: vshPrintExtra(ctl, _("Domain %s started\n"), virDomainGetName(dom)); #ifndef WIN32

On Wed, Jun 13, 2018 at 12:53 AM +0200, John Ferlan <jferlan@redhat.com> wrote:
On 05/09/2018 10:56 AM, Marc Hartmayer wrote:
The force boot emulation is only required for virDomainCreateWithFlags as the flag VIR_DOMAIN_START_FORCE_BOOT was introduced with the commit 27c85260532f879be5674a4eed0811c21fd34f94 (2011). But virDomainCreateXMLWithFiles is newer and therefore already had support for VIR_DOMAIN_START_FORCE_BOOT. This means there is now no second call with VIR_DOMAIN_START_FORCE_BOOT removed from flags for virDomainCreateXMLWithFiles in case the first call fails. virDomainCreateXMLWithFiles was introduced with commit d76227bea35cc49374a94414f6d46e3493ac2a52 (2013).
This patch takes this into account and simplifies the function. In addition, it's now easier to extend the function.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- tools/virsh-domain.c | 52 ++++++++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 24 deletions(-)
OK so I took the bait ;-)... I agree 110% what you've changed to is easier to read; however, I think there's a subtle difference with your changes... I'm not sure this new flow will work quite right "if" for some reason someone passes the --force-boot flag to/for a startup that uses CreateWithFiles, such as lxc.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 598d2fa4a4bd..7cf8373f05bc 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4038,40 +4038,44 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "force-boot")) flags |= VIR_DOMAIN_START_FORCE_BOOT;> - /* We can emulate force boot, even for older servers that reject it. */
FWIW: I see that this hunk was added by commit id 691ec08b shortly after adding support for force_boot via commit id 27c85260.
- if (flags & VIR_DOMAIN_START_FORCE_BOOT) { - if ((nfds ? - virDomainCreateWithFiles(dom, nfds, fds, flags) : - virDomainCreateWithFlags(dom, flags)) == 0) - goto started; - if (last_error->code != VIR_ERR_NO_SUPPORT && - last_error->code != VIR_ERR_INVALID_ARG) { - vshReportError(ctl); - goto cleanup; - }
Previously if flags & VIR_DOMAIN_START_FORCE_BOOT, we'd try w/ CreateWithFiles without changing @flags.
For lxc, eventually we'd end up in lxcDomainCreateWithFiles which would do a virCheckFlags and "fail" with VIR_ERR_INVALID_ARG because the flag VIR_DOMAIN_START_FORCE_BOOT is not supported.
Okay. I haven’t thought that a newer API wouldn’t support this flag.
That would fall through this removed code and remove the FORCE_BOOT flag. Then we'd again call virDomainCreateWithFiles w/ @flags adjusted to remove VIR_DOMAIN_START_FORCE_BOOT
- vshResetLibvirtError(); - rc = virDomainHasManagedSaveImage(dom, 0); - if (rc < 0) { - /* No managed save image to remove */ - vshResetLibvirtError(); - } else if (rc > 0) { - if (virDomainManagedSaveRemove(dom, 0) < 0) { + /* Prefer older API unless we have to pass extra parameters */ + if (nfds) { + rc = virDomainCreateWithFiles(dom, nfds, fds, flags);
With this new code if @flags has VIR_DOMAIN_START_FORCE_BOOT, then there's no fallback and we fail.
+ } else if (flags) { + rc = virDomainCreateWithFlags(dom, flags); + /* We can emulate force boot, even for older servers that + * reject it. */ + if (rc < 0 && flags & VIR_DOMAIN_START_FORCE_BOOT) { + if (last_error->code != VIR_ERR_NO_SUPPORT && + last_error->code != VIR_ERR_INVALID_ARG) {
The CreateWithFlags code could "fail" w/ INVALID_ARG if some hypervisor didn't support some flag, but even calling it again without the FORCE_BOOT may not change the result.
This (existing, I know) code assumes the failure is from the lack of a FORCE_BOOT flag as added via commit 691ec08b. Having commit id afb50d79 further complicate the logic especially w/r/t FORCE_BOOT (which I think makes no sense for a container hypervisor, but then again I'm not the expert there ;-))
:)
. Still since it's in the API and documented, it's not like we could remove it (whether or not it was an unintentional cut-copy-paste, change API name).
Anyway, thanks for the review! […snip] -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

Introduce new libvirt API virDomainCreateWithParams that allows to temporarily boot from another boot device, to use another kernel, initrd, and cmdline than defined in the persistent domain definition. All typed parameters are optional. The design of the API was chosen to ease future extensions. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- include/libvirt/libvirt-domain.h | 37 +++++++++++++++++++++++ src/driver-hypervisor.h | 6 ++++ src/libvirt-domain.c | 64 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 4 +++ 4 files changed, 111 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 12fd34037e5d..58e4de048a4f 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1790,6 +1790,43 @@ int virConnectListAllDomains (virConnectPtr conn, virDomainPtr **domains, unsigned int flags); int virDomainCreate (virDomainPtr domain); + +/** + * VIR_DOMAIN_CREATE_PARM_KERNEL: + * + * Macro for typed parameter name that represents the used kernel. It + * corresponds to the "kernel" node in the XML. + */ +# define VIR_DOMAIN_CREATE_PARM_KERNEL "kernel" + +/** + * VIR_DOMAIN_CREATE_PARM_CMDLINE: + * + * Macro for typed parameter name that represents the used cmdline. It + * corresponds to the "cmdline" node in the XML. + */ +# define VIR_DOMAIN_CREATE_PARM_CMDLINE "cmdline" + +/** + * VIR_DOMAIN_CREATE_PARM_INITRD: + * + * Macro for typed parameter name that represents the used initial + * ramdisk. It corresponds to the "initrd" node in the XML. + */ +# define VIR_DOMAIN_CREATE_PARM_INITRD "initrd" + +/** + * VIR_DOMAIN_CREATE_PARM_DEVICE_IDENTIFIER: + * + * Macro for typed parameter name that represents the identifier for + * the boot device. + */ +# define VIR_DOMAIN_CREATE_PARM_DEVICE_IDENTIFIER "bootdevice" + +int virDomainCreateWithParams (virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags); int virDomainCreateWithFlags (virDomainPtr domain, unsigned int flags); diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index e71a72a44132..2fa9e09e12f6 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -313,6 +313,11 @@ typedef int unsigned int nfiles, int *files, unsigned int flags); +typedef int +(*virDrvDomainCreateWithParams)(virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + unsigned int flags); typedef virDomainPtr (*virDrvDomainDefineXML)(virConnectPtr conn, @@ -1383,6 +1388,7 @@ struct _virHypervisorDriver { virDrvDomainCreate domainCreate; virDrvDomainCreateWithFlags domainCreateWithFlags; virDrvDomainCreateWithFiles domainCreateWithFiles; + virDrvDomainCreateWithParams domainCreateWithParams; virDrvDomainDefineXML domainDefineXML; virDrvDomainDefineXMLFlags domainDefineXMLFlags; virDrvDomainUndefine domainUndefine; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 2d86e48979d3..36badab2d5d9 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -6682,6 +6682,70 @@ virDomainCreateWithFiles(virDomainPtr domain, unsigned int nfiles, /** + * virDomainCreateWithParams: + * @domain: pointer to a defined domain + * @params: pointer to boot parameter objects + * @nparams: number of boot parameter objects + * @flags: bitwise-OR of supported virDomainCreateFlags + * + * Launch a defined domain. If the call succeeds the domain moves from + * the defined to the running domains pools. + * + * @params provides an array of typed parameters with the length + * @nparams. This array will be used to configure the domain to be + * temporary started from the device specified by the typed parameter + * 'bootdevice'. With the typed parameters 'kernel', 'initrd', and + * 'cmdline' it's possible to temporarily override the corresponding + * values. All typed parameters are optional. + * + * For more control over @flags, see virDomainCreateWithFlags(). + * + * Returns 0 in case of success, -1 in case of error + */ +int +virDomainCreateWithParams(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, flags=0x%x", + params, nparams, flags); + VIR_TYPED_PARAMS_DEBUG(params, nparams); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + virCheckNonNegativeArgGoto(nparams, error); + if (nparams > 0) + virCheckNonNullArgGoto(params, error); + + if (virTypedParameterValidateSet(conn, params, nparams) < 0) + goto error; + + if (conn->driver->domainCreateWithParams) { + int ret; + ret = conn->driver->domainCreateWithParams(domain, + params, + nparams, + flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + error: + virDispatchError(domain->conn); + return -1; +} + + +/** * virDomainGetAutostart: * @domain: a domain object * @autostart: the value returned diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 95df3a0dbc7b..5d9b2697702c 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -785,4 +785,8 @@ LIBVIRT_4.1.0 { virStoragePoolLookupByTargetPath; } LIBVIRT_3.9.0; +LIBVIRT_4.4.0 { + global: + virDomainCreateWithParams; +} LIBVIRT_4.1.0; # .... define new API here using predicted next version number .... -- 2.13.4

On Wed, May 09, 2018 at 04:56:12PM +0200, Marc Hartmayer wrote:
Introduce new libvirt API virDomainCreateWithParams that allows to temporarily boot from another boot device, to use another kernel, initrd, and cmdline than defined in the persistent domain definition. All typed parameters are optional.
The design of the API was chosen to ease future extensions.
I don't really see the point in doing this. We already have the ability to temporary boot with a different configuration than is stored in the persistent XML. Just call virDomainCreateXML() passing in the alternative XML doc. This allows changing *any* aspect of the guest configuration, so we're not restricted to just bot device, kernel initrd and cmdline, and thus won't need to write more code anytime someone asks to be able to override something else too. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Wed, May 09, 2018 at 05:40 PM +0200, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
On Wed, May 09, 2018 at 04:56:12PM +0200, Marc Hartmayer wrote:
Introduce new libvirt API virDomainCreateWithParams that allows to temporarily boot from another boot device, to use another kernel, initrd, and cmdline than defined in the persistent domain definition. All typed parameters are optional.
The design of the API was chosen to ease future extensions.
I don't really see the point in doing this. We already have the ability to temporary boot with a different configuration than is stored in the persistent XML. Just call virDomainCreateXML() passing in the alternative XML doc. This allows changing *any* aspect of the guest configuration, so we're not restricted to just bot device, kernel initrd and cmdline, and thus won't need to write more code anytime someone asks to be able to override something else too.
I know there is the API virDomainCreateXML for creating a transient domain and that it’s possible to temporarily replace parts of the persistent XML with it. But my idea is _not_ to add a functionality to override parts of the persistent XML. My idea is to provide support allowing an easy one-time switch of the boot device in a persistently defined domain. For s390 it’s essential to have an easy way to change the boot configuration since it “knows” only one boot device at a time and it has no support for interactively changing the boot device during the boot/IPL process. I started out with a fixed API (as it was done for example with virDomanCreateWithFiles) but than I liked the approach of providing an more extensible and flexible API better. This is also the reason why I used typed parameters for passing the arguments. This type of API is also not restricted to boot order changes since it could be freely expanded (e.g. passing file descriptors). I can certainly revert back to the static API.
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
-- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On Wed, May 16, 2018 at 05:30:33PM +0200, Marc Hartmayer wrote:
On Wed, May 09, 2018 at 05:40 PM +0200, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
On Wed, May 09, 2018 at 04:56:12PM +0200, Marc Hartmayer wrote:
Introduce new libvirt API virDomainCreateWithParams that allows to temporarily boot from another boot device, to use another kernel, initrd, and cmdline than defined in the persistent domain definition. All typed parameters are optional.
The design of the API was chosen to ease future extensions.
I don't really see the point in doing this. We already have the ability to temporary boot with a different configuration than is stored in the persistent XML. Just call virDomainCreateXML() passing in the alternative XML doc. This allows changing *any* aspect of the guest configuration, so we're not restricted to just bot device, kernel initrd and cmdline, and thus won't need to write more code anytime someone asks to be able to override something else too.
I know there is the API virDomainCreateXML for creating a transient domain and that it’s possible to temporarily replace parts of the persistent XML with it. But my idea is _not_ to add a functionality to override parts of the persistent XML. My idea is to provide support allowing an easy one-time switch of the boot device in a persistently defined domain. For s390 it’s essential to have an easy way to change the boot configuration since it “knows” only one boot device at a time and it has no support for interactively changing the boot device during the boot/IPL process.
virDomainCreateXML does not change the persistent XML. If you have an existing persistent guest, and use virDomainCreateXML it lets you start that guest with that customized XML just once, without affecting the persistent XML at all.
I started out with a fixed API (as it was done for example with virDomanCreateWithFiles) but than I liked the approach of providing an more extensible and flexible API better. This is also the reason why I used typed parameters for passing the arguments. This type of API is also not restricted to boot order changes since it could be freely expanded (e.g. passing file descriptors). I can certainly revert back to the static API.
Using virDomainCreateXML is the most flexible API because it lets you changed anything in the XML for just that one boot up attempt. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 05/16/2018 05:35 PM, Daniel P. Berrangé wrote:
On Wed, May 16, 2018 at 05:30:33PM +0200, Marc Hartmayer wrote:
On Wed, May 09, 2018 at 05:40 PM +0200, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
On Wed, May 09, 2018 at 04:56:12PM +0200, Marc Hartmayer wrote:
Introduce new libvirt API virDomainCreateWithParams that allows to temporarily boot from another boot device, to use another kernel, initrd, and cmdline than defined in the persistent domain definition. All typed parameters are optional.
The design of the API was chosen to ease future extensions.
I don't really see the point in doing this. We already have the ability to temporary boot with a different configuration than is stored in the persistent XML. Just call virDomainCreateXML() passing in the alternative XML doc. This allows changing *any* aspect of the guest configuration, so we're not restricted to just bot device, kernel initrd and cmdline, and thus won't need to write more code anytime someone asks to be able to override something else too.
I know there is the API virDomainCreateXML for creating a transient domain and that it’s possible to temporarily replace parts of the persistent XML with it. But my idea is _not_ to add a functionality to override parts of the persistent XML. My idea is to provide support allowing an easy one-time switch of the boot device in a persistently defined domain. For s390 it’s essential to have an easy way to change the boot configuration since it “knows” only one boot device at a time and it has no support for interactively changing the boot device during the boot/IPL process.
virDomainCreateXML does not change the persistent XML. If you have an existing persistent guest, and use virDomainCreateXML it lets you start that guest with that customized XML just once, without affecting the persistent XML at all.
I started out with a fixed API (as it was done for example with virDomanCreateWithFiles) but than I liked the approach of providing an more extensible and flexible API better. This is also the reason why I used typed parameters for passing the arguments. This type of API is also not restricted to boot order changes since it could be freely expanded (e.g. passing file descriptors). I can certainly revert back to the static API.
Using virDomainCreateXML is the most flexible API because it lets you changed anything in the XML for just that one boot up attempt.
To clarify this: would it be ok for you to implement this feature (the command line parameter to "virsh start") in virsh by doing the xml mangling in the virsh command line tool itself?

On Wed, May 16, 2018 at 06:21:40PM +0200, Christian Borntraeger wrote:
On 05/16/2018 05:35 PM, Daniel P. Berrangé wrote:
On Wed, May 16, 2018 at 05:30:33PM +0200, Marc Hartmayer wrote:
On Wed, May 09, 2018 at 05:40 PM +0200, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
On Wed, May 09, 2018 at 04:56:12PM +0200, Marc Hartmayer wrote:
Introduce new libvirt API virDomainCreateWithParams that allows to temporarily boot from another boot device, to use another kernel, initrd, and cmdline than defined in the persistent domain definition. All typed parameters are optional.
The design of the API was chosen to ease future extensions.
I don't really see the point in doing this. We already have the ability to temporary boot with a different configuration than is stored in the persistent XML. Just call virDomainCreateXML() passing in the alternative XML doc. This allows changing *any* aspect of the guest configuration, so we're not restricted to just bot device, kernel initrd and cmdline, and thus won't need to write more code anytime someone asks to be able to override something else too.
I know there is the API virDomainCreateXML for creating a transient domain and that it’s possible to temporarily replace parts of the persistent XML with it. But my idea is _not_ to add a functionality to override parts of the persistent XML. My idea is to provide support allowing an easy one-time switch of the boot device in a persistently defined domain. For s390 it’s essential to have an easy way to change the boot configuration since it “knows” only one boot device at a time and it has no support for interactively changing the boot device during the boot/IPL process.
virDomainCreateXML does not change the persistent XML. If you have an existing persistent guest, and use virDomainCreateXML it lets you start that guest with that customized XML just once, without affecting the persistent XML at all.
I started out with a fixed API (as it was done for example with virDomanCreateWithFiles) but than I liked the approach of providing an more extensible and flexible API better. This is also the reason why I used typed parameters for passing the arguments. This type of API is also not restricted to boot order changes since it could be freely expanded (e.g. passing file descriptors). I can certainly revert back to the static API.
Using virDomainCreateXML is the most flexible API because it lets you changed anything in the XML for just that one boot up attempt.
To clarify this: would it be ok for you to implement this feature (the command line parameter to "virsh start") in virsh by doing the xml mangling in the virsh command line tool itself?
I'm not really a fan of that, as the modifications to virsh are just as non-extensible as the modifications to the API. Each time someone wants another field customizable, we have to add new virsh CLI args and XML munging for them. I could suggest a general arg 'virsh start --edit $GUESTNAME' though, which takes the current persistent XML, launches it in an editor, allowing the user to change the boot order and then starts the guest from this temporary editted XML using virDomainCreateXML. This is a conceptual fit with the 'virsh edit $GUESTNAME' command we already have Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Wednesday, 16 May 2018 18:31:26 CEST Daniel P. Berrangé wrote:
On Wed, May 16, 2018 at 06:21:40PM +0200, Christian Borntraeger wrote:
On 05/16/2018 05:35 PM, Daniel P. Berrangé wrote:
On Wed, May 16, 2018 at 05:30:33PM +0200, Marc Hartmayer wrote:
On Wed, May 09, 2018 at 05:40 PM +0200, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
On Wed, May 09, 2018 at 04:56:12PM +0200, Marc Hartmayer wrote:
Introduce new libvirt API virDomainCreateWithParams that allows to temporarily boot from another boot device, to use another kernel, initrd, and cmdline than defined in the persistent domain definition. All typed parameters are optional.
The design of the API was chosen to ease future extensions.
I don't really see the point in doing this. We already have the ability to temporary boot with a different configuration than is stored in the persistent XML. Just call virDomainCreateXML() passing in the alternative XML doc. This allows changing *any* aspect of the guest configuration, so we're not restricted to just bot device, kernel initrd and cmdline, and thus won't need to write more code anytime someone asks to be able to override something else too.
I know there is the API virDomainCreateXML for creating a transient domain and that it’s possible to temporarily replace parts of the persistent XML with it. But my idea is _not_ to add a functionality to override parts of the persistent XML. My idea is to provide support allowing an easy one-time switch of the boot device in a persistently defined domain. For s390 it’s essential to have an easy way to change the boot configuration since it “knows” only one boot device at a time and it has no support for interactively changing the boot device during the boot/IPL process.
virDomainCreateXML does not change the persistent XML. If you have an existing persistent guest, and use virDomainCreateXML it lets you start that guest with that customized XML just once, without affecting the persistent XML at all.
I started out with a fixed API (as it was done for example with virDomanCreateWithFiles) but than I liked the approach of providing an more extensible and flexible API better. This is also the reason why I used typed parameters for passing the arguments. This type of API is also not restricted to boot order changes since it could be freely expanded (e.g. passing file descriptors). I can certainly revert back to the static API.
Using virDomainCreateXML is the most flexible API because it lets you changed anything in the XML for just that one boot up attempt.
To clarify this: would it be ok for you to implement this feature (the command line parameter to "virsh start") in virsh by doing the xml mangling in the virsh command line tool itself?
I'm not really a fan of that, as the modifications to virsh are just as non-extensible as the modifications to the API. Each time someone wants another field customizable, we have to add new virsh CLI args and XML munging for them.
I could suggest a general arg 'virsh start --edit $GUESTNAME' though, which takes the current persistent XML, launches it in an editor, allowing the user to change the boot order and then starts the guest from this temporary editted XML using virDomainCreateXML. This is a conceptual fit with the 'virsh edit $GUESTNAME' command we already have
Another option could be instead to implement this "temporarly change & start" to another tool, that already does XML munging: virt-xml. In that context, adding options to edit the various bits makes sense, so only the "edit -> start -> revert" thing would be needed there. WDYT? -- Pino Toscano

On Wed, Jun 06, 2018 at 02:53 PM +0200, Pino Toscano <ptoscano@redhat.com> wrote:
On Wednesday, 16 May 2018 18:31:26 CEST Daniel P. Berrangé wrote:
On Wed, May 16, 2018 at 06:21:40PM +0200, Christian Borntraeger wrote:
On 05/16/2018 05:35 PM, Daniel P. Berrangé wrote:
[…snip…]
I could suggest a general arg 'virsh start --edit $GUESTNAME' though, which takes the current persistent XML, launches it in an editor, allowing the user to change the boot order and then starts the guest from this temporary editted XML using virDomainCreateXML. This is a conceptual fit with the 'virsh edit $GUESTNAME' command we already have
Another option could be instead to implement this "temporarly change & start" to another tool, that already does XML munging: virt-xml. In that context, adding options to edit the various bits makes sense, so only the "edit -> start -> revert" thing would be needed there.
WDYT?
Wouldn’t be a (new) tool 'virt-start' better for that since virt-xml does only the XML manipulation? Thanks for thinking about it.
-- Pino Toscano -- Beste Grüße / Kind regards Marc Hartmayer
IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

Add support for virDomainCreateWithParams to the remote driver. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 22 +++++++++++++++++++++- src/remote_protocol-structs | 12 ++++++++++++ src/rpc/gendispatch.pl | 18 ++++++++++++------ 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 95437b43657e..1bbf42c1e1b2 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8297,6 +8297,7 @@ static virHypervisorDriver hypervisor_driver = { .connectListDefinedDomains = remoteConnectListDefinedDomains, /* 0.3.0 */ .connectNumOfDefinedDomains = remoteConnectNumOfDefinedDomains, /* 0.3.0 */ .domainCreate = remoteDomainCreate, /* 0.3.0 */ + .domainCreateWithParams = remoteDomainCreateWithParams, /* 4.4.0 */ .domainCreateWithFlags = remoteDomainCreateWithFlags, /* 0.8.2 */ .domainCreateWithFiles = remoteDomainCreateWithFiles, /* 1.1.1 */ .domainDefineXML = remoteDomainDefineXML, /* 0.3.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 296a0871813a..d22299879429 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -73,6 +73,9 @@ const REMOTE_IOTHREAD_INFO_MAX = 16384; /* Upper limit on migrate cookie. */ const REMOTE_MIGRATE_COOKIE_MAX = 4194304; +/* Upper limit on create params. */ +const REMOTE_CREATE_PARAMS_MAX = 4; + /* Upper limit on lists of networks. */ const REMOTE_NETWORK_LIST_MAX = 16384; @@ -1054,6 +1057,16 @@ struct remote_domain_create_args { remote_nonnull_domain dom; }; +struct remote_domain_create_with_params_args { + remote_nonnull_domain dom; + remote_typed_param params<REMOTE_CREATE_PARAMS_MAX>; + unsigned int flags; +}; + +struct remote_domain_create_with_params_ret { + remote_nonnull_domain dom; +}; + struct remote_domain_create_with_flags_args { remote_nonnull_domain dom; unsigned int flags; @@ -6135,5 +6148,12 @@ enum remote_procedure { * @priority: high * @acl: storage_pool:getattr */ - REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391 + REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391, + + /** + * @generate: both + * @acl: domain:start + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_CREATE_WITH_PARAMS = 392 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index fe163db73f88..49ef88f07e62 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -684,6 +684,17 @@ struct remote_connect_num_of_defined_domains_ret { struct remote_domain_create_args { remote_nonnull_domain dom; }; +struct remote_domain_create_with_params_args { + remote_nonnull_domain dom; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + u_int flags; +}; +struct remote_domain_create_with_params_ret { + remote_nonnull_domain dom; +}; struct remote_domain_create_with_flags_args { remote_nonnull_domain dom; u_int flags; @@ -3269,4 +3280,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_MANAGED_SAVE_DEFINE_XML = 389, REMOTE_PROC_DOMAIN_SET_LIFECYCLE_ACTION = 390, REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391, + REMOTE_PROC_DOMAIN_CREATE_WITH_PARAMS = 392, }; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index b8b83b6b40d3..1e32c10e7b7a 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -783,9 +783,12 @@ elsif ($mode eq "server") { } elsif ($ret_member =~ m/^remote_nonnull_(domain|network|storage_pool|storage_vol|interface|node_device|secret|nwfilter|domain_snapshot) (\S+);/) { my $type_name = name_to_TypeName($1); - if ($call->{ProcName} eq "DomainCreateWithFlags") { - # SPECIAL: virDomainCreateWithFlags updates the given - # domain object instead of returning a new one + if ($call->{ProcName} eq "DomainCreateWithFlags" || + $call->{ProcName} eq "DomainCreateWithParams") { + # SPECIAL: virDomainCreateWithFlags, + # virDomainCreateWithParams, + # updates the given domain object instead of + # returning a new one push(@ret_list, "make_nonnull_$1(&ret->$2, $2);"); $single_ret_var = undef; $single_ret_by_ref = 1; @@ -1576,9 +1579,12 @@ elsif ($mode eq "client") { my $arg_name = $2; my $type_name = name_to_TypeName($name); - if ($call->{ProcName} eq "DomainCreateWithFlags") { - # SPECIAL: virDomainCreateWithFlags updates the given - # domain object instead of returning a new one + if ($call->{ProcName} eq "DomainCreateWithFlags" || + $call->{ProcName} eq "DomainCreateWithParams") { + # SPECIAL: virDomainCreateWithFlags, + # virDomainCreateWithParams + # updates the given domain object instead of + # returning a new one push(@ret_list, "dom->id = ret.dom.id;"); push(@ret_list, "xdr_free((xdrproc_t)xdr_$call->{ret}, (char *)&ret);"); push(@ret_list, "rv = 0;"); -- 2.13.4

If @src is an empty string then @dst will be freed otherwise @dst will be a duplicate of @src. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> --- src/libvirt_private.syms | 1 + src/util/virstring.c | 27 +++++++++++++++++++++++++++ src/util/virstring.h | 2 ++ 3 files changed, 30 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f8883dc50da2..c684f21905af 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2871,6 +2871,7 @@ virStringStripControlChars; virStringStripIPv6Brackets; virStringToUpper; virStringTrimOptionalNewline; +virStringUpdate; virStrncpy; virStrndup; virStrToDouble; diff --git a/src/util/virstring.c b/src/util/virstring.c index 15f367af7c18..1e9bf1f561b5 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1248,6 +1248,33 @@ virStringReplace(const char *haystack, /** + * virStringUpdate: + * @dest: where to update string + * @src: string to duplicate + * + * If @src is an empty string then @dest will be freed otherwise @dest + * will be a duplicate of @src. + * + * Returns -1 on failure (with OOM error reported), 0 if @dest was + * freed only, 1 if @src was copied + */ +int +virStringUpdate(char **dest, const char *src) +{ + if (virStringIsEmpty(src)) { + VIR_FREE(*dest); + return 0; + } else { + char *temp = *dest; + if (VIR_STRDUP(*dest, src) < 0) + return -1; + VIR_FREE(temp); + return 1; + } +} + + +/** * virStringStripIPv6Brackets: * @str: the string to strip * diff --git a/src/util/virstring.h b/src/util/virstring.h index fa2ec1df4def..69f2cb69037d 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -290,6 +290,8 @@ char *virStringReplace(const char *haystack, const char *oldneedle, const char *newneedle) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +int virStringUpdate(char **dest, const char *src) + ATTRIBUTE_NONNULL(1); void virStringStripIPv6Brackets(char *str); bool virStringHasChars(const char *str, -- 2.13.4

Add functions for changing the boot configuration of a domain in preparation for upcoming patches. It's possible to change the used kernel, initrd, cmdline, and the used boot device. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/conf/domain_conf.c | 226 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 11 +++ src/libvirt_private.syms | 1 + 3 files changed, 238 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f678e26b28fd..326afa196cc6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -28712,6 +28712,232 @@ virDomainObjSetMetadata(virDomainObjPtr vm, } +/** + * virDomainDefSetBootDevice: + * @def: domain definition + * @device: set the boot order priority of this device to highest, + * the domain must contain this device + * + * Set @device as the new boot device for @def + * + * Increment all boot indices that are set (bootindex > 0) except for + * @device and all those devices that have a bootindex greater than + * the old bootindex of @device if the index was set. Additionally, it + * disables the boot order defined in the 'os' XML node of a domain + * definition. + * + * E.g. a domain has the following devices + * diskA: boot order 1 + * diskB: boot order 2 + * hostdevA: boot order 3 + * If we now set diskB to the new boot device. It results in + * diskA: boot order 2 + * diskB: boot order 1 + * hostdevA: boot order 3 (note: the boot order value remains the same) + * + * Returns 0 in case of success, -1 in case of error + */ +static int +virDomainDefSetBootDevice(virDomainDefPtr def, + virDomainDeviceDefPtr device) +{ + size_t i; + unsigned int oldBootIndex = 0; + virDomainDeviceInfoPtr info; + bool noOldBootIndex; + + /* Set nBootDevs to 0 so the per-device boot configuration will be + * used. def->os.bootDevs does not have to be freed here. */ + def->os.nBootDevs = 0; + + switch ((virDomainDeviceType) device->type) { + case VIR_DOMAIN_DEVICE_DISK: + case VIR_DOMAIN_DEVICE_NET: + case VIR_DOMAIN_DEVICE_HOSTDEV: + case VIR_DOMAIN_DEVICE_REDIRDEV: + info = virDomainDeviceGetInfo(device); + if (!info) { + virReportError(VIR_ERR_OPERATION_FAILED, + "%s", + _("No device information found for the selected device")); + return -1; + } + oldBootIndex = info->bootIndex; + info->bootIndex = 1; + break; + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_IOMMU: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("It's not supported to boot from the selected device type '%s'"), + virDomainDeviceTypeToString(device->type)); + return -1; + } + + noOldBootIndex = oldBootIndex == 0; + + /* A precondition is that all the boot indexes must be numbered + consecutively or not defined. This is the case here. We must + change the boot index only of those devices that are affected + by the change of the old boot index. There are only two cases + were this is true: + + 1. Not the same device and it uses a boot index (> 0) and which + is less than the oldBootIndex + 2. Not the same device and it uses a boot index (> 0) and @device + used no boot index + */ + for (i = 0; i < def->ndisks; i++) { + if (device->type == VIR_DOMAIN_DEVICE_DISK && + def->disks[i] == device->data.disk) + continue; + + if (def->disks[i]->info.bootIndex > 0 && + (def->disks[i]->info.bootIndex < oldBootIndex || + noOldBootIndex)) + def->disks[i]->info.bootIndex++; + } + + for (i = 0; i < def->nnets; i++) { + if (device->type == VIR_DOMAIN_DEVICE_NET && + def->nets[i] == device->data.net) + continue; + + if (def->nets[i]->info.bootIndex > 0 && + (def->nets[i]->info.bootIndex < oldBootIndex || + noOldBootIndex)) + def->nets[i]->info.bootIndex++; + } + + for (i = 0; i < def->nhostdevs; i++) { + if (device->type == VIR_DOMAIN_DEVICE_HOSTDEV && + def->hostdevs[i] == device->data.hostdev) + continue; + + if (def->hostdevs[i]->info->bootIndex > 0 && + (def->hostdevs[i]->info->bootIndex < oldBootIndex || + noOldBootIndex)) + def->hostdevs[i]->info->bootIndex++; + } + + for (i = 0; i < def->nredirdevs; i++) { + if (device->type == VIR_DOMAIN_DEVICE_REDIRDEV && + def->redirdevs[i] == device->data.redirdev) + continue; + + if (def->redirdevs[i]->info.bootIndex > 0 && + (def->redirdevs[i]->info.bootIndex < oldBootIndex || + noOldBootIndex)) + def->redirdevs[i]->info.bootIndex++; + } + + return 0; +} + + +/** + * virDomainDefSetBootDeviceByIdentifier: + * @def: Domain definition + * @bootDeviceIdentifier: Selector for the device. + * Currently only disk and network devices are supported. + * + * Returns 0 in case of success, -1 in case of error. + */ +static int +virDomainDefSetBootDeviceByIdentifier(virDomainDefPtr def, + const char *bootDeviceIdentifier) +{ + virDomainDiskDefPtr diskDef = NULL; + virDomainNetDefPtr netDef = NULL; + virDomainDeviceDef bootDevice; + + if (!bootDeviceIdentifier) + return 0; + + /* Look for the correct device. At first try to find a disk with + * this device name, if not found try to find a network device + * with this device name. Disk devices are identified by '<target + * dev="name"...' dev value. Network devices are identified by + * their MAC address value. + */ + if ((diskDef = virDomainDiskByName(def, bootDeviceIdentifier, false)) != NULL) { + bootDevice = (virDomainDeviceDef) { + .type = VIR_DOMAIN_DEVICE_DISK, + .data.disk = diskDef + }; + } else if ((netDef = virDomainNetFind(def, bootDeviceIdentifier)) != NULL) { + bootDevice = (virDomainDeviceDef) { + .type = VIR_DOMAIN_DEVICE_NET, + .data.net = netDef + }; + } else { + virReportError(VIR_ERR_OPERATION_FAILED, + _("no device matching '%s' found"), + bootDeviceIdentifier); + return -1; + } + + return virDomainDefSetBootDevice(def, &bootDevice); +} + + +/** + * virDomainDefOverrideBootConf: + * @def: Domain definition + * @params: Parameters that will be used to override the current boot configuration. + * + * This function allows to override the used boot device, kernel, + * initial ramdisk, and the cmdline of the given domain + * definition. The parameters for overwriting are passed via @params. + * If the passed kernel, initrd, or cmdline is an empty string the + * original value of the domain definition will be freed. + * + * Returns 0 in case of success, -1 in case of error. + */ +int +virDomainDefOverrideBootConf(virDomainDefPtr def, + const virCreateParams *params) +{ + virDomainOSDefPtr os = &def->os; + + if (!params) + return 0; + + if (params->bootDeviceIdentifier && + virDomainDefSetBootDeviceByIdentifier(def, params->bootDeviceIdentifier) < 0) + return -1; + + if (params->kernel && virStringUpdate(&os->kernel, params->kernel) < 0) + return -1; + + if (params->initrd && virStringUpdate(&os->initrd, params->initrd) < 0) + return -1; + + if (params->cmdline && virStringUpdate(&os->cmdline, params->cmdline) < 0) + return -1; + + return 0; +} + + bool virDomainDefNeedsPlacementAdvice(virDomainDefPtr def) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 15d228ba9e62..87c14dc9b2e7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3420,6 +3420,17 @@ int virDomainObjSetMetadata(virDomainObjPtr vm, const char *configDir, unsigned int flags); +typedef struct { + const char *bootDeviceIdentifier; + const char *kernel; + const char *initrd; + const char *cmdline; +} virCreateParams; + +int +virDomainDefOverrideBootConf(virDomainDefPtr def, + const virCreateParams *params); + int virDomainParseMemory(const char *xpath, const char *units_xpath, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c684f21905af..d3e30beb87c0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -285,6 +285,7 @@ virDomainDefMaybeAddInput; virDomainDefNeedsPlacementAdvice; virDomainDefNew; virDomainDefNewFull; +virDomainDefOverrideBootConf; virDomainDefParseFile; virDomainDefParseNode; virDomainDefParseString; -- 2.13.4

Add the functionality to override the boot configuration of a domain at the start time to the QEMU driver. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> --- src/qemu/qemu_driver.c | 36 ++++++++++++++++++++++++++---------- src/qemu/qemu_migration.c | 3 ++- src/qemu/qemu_process.c | 16 +++++++++++----- src/qemu/qemu_process.h | 2 ++ 4 files changed, 41 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c129321a5c25..9a43d4157b09 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -145,7 +145,8 @@ static int qemuDomainObjStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned int flags, - qemuDomainAsyncJob asyncJob); + qemuDomainAsyncJob asyncJob, + const virCreateParams *createParams); static int qemuDomainManagedSaveLoad(virDomainObjPtr vm, void *opaque); @@ -257,6 +258,7 @@ qemuAutostartDomain(virDomainObjPtr vm, int flags = 0; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); int ret = -1; + const virCreateParams createParams = { 0 }; if (cfg->autoStartBypassCache) flags |= VIR_DOMAIN_START_BYPASS_CACHE; @@ -275,7 +277,7 @@ qemuAutostartDomain(virDomainObjPtr vm, } if (qemuDomainObjStart(NULL, driver, vm, flags, - QEMU_ASYNC_JOB_START) < 0) { + QEMU_ASYNC_JOB_START, &createParams) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to autostart VM '%s': %s"), vm->def->name, virGetLastErrorMessage()); @@ -1735,6 +1737,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, virCapsPtr caps = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; + const virCreateParams createParams = { 0 }; virCheckFlags(VIR_DOMAIN_START_PAUSED | VIR_DOMAIN_START_AUTODESTROY | @@ -1776,7 +1779,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, if (qemuProcessStart(conn, driver, vm, NULL, QEMU_ASYNC_JOB_START, NULL, -1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, - start_flags) < 0) { + &createParams, start_flags) < 0) { virDomainAuditStart(vm, "booted", false); qemuDomainRemoveInactive(driver, vm); qemuProcessEndJob(driver, vm); @@ -6558,6 +6561,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virQEMUSaveHeaderPtr header = &data->header; qemuDomainSaveCookiePtr cookie = NULL; + const virCreateParams createParams = { 0 }; if (virSaveCookieParseString(data->cookie, (virObjectPtr *)&cookie, virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0) @@ -6592,7 +6596,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, if (qemuProcessStart(conn, driver, vm, cookie ? cookie->cpu : NULL, asyncJob, "stdio", *fd, path, NULL, VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, - VIR_QEMU_PROCESS_START_PAUSED) == 0) + &createParams, VIR_QEMU_PROCESS_START_PAUSED) == 0) restored = true; if (intermediatefd != -1) { @@ -7234,7 +7238,8 @@ qemuDomainObjStart(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned int flags, - qemuDomainAsyncJob asyncJob) + qemuDomainAsyncJob asyncJob, + const virCreateParams *createParams) { int ret = -1; char *managed_save; @@ -7270,6 +7275,13 @@ qemuDomainObjStart(virConnectPtr conn, virDomainJobOperation op = priv->job.current->operation; priv->job.current->operation = VIR_DOMAIN_JOB_OPERATION_RESTORE; + if (createParams->bootDeviceIdentifier || createParams->kernel || + createParams->initrd || createParams->cmdline) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Transient changes are not possible with the managed save option.")); + goto cleanup; + } + ret = qemuDomainObjRestore(conn, driver, vm, managed_save, start_paused, bypass_cache, asyncJob); @@ -7294,7 +7306,8 @@ qemuDomainObjStart(virConnectPtr conn, ret = qemuProcessStart(conn, driver, vm, NULL, asyncJob, NULL, -1, NULL, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags); + VIR_NETDEV_VPORT_PROFILE_OP_CREATE, + createParams, start_flags); virDomainAuditStart(vm, "booted", ret >= 0); if (ret >= 0) { virObjectEventPtr event = @@ -7317,12 +7330,14 @@ qemuDomainObjStart(virConnectPtr conn, return ret; } + static int qemuDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; + const virCreateParams createParams = { 0 }; virCheckFlags(VIR_DOMAIN_START_PAUSED | VIR_DOMAIN_START_AUTODESTROY | @@ -7348,7 +7363,7 @@ qemuDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) } if (qemuDomainObjStart(dom->conn, driver, vm, flags, - QEMU_ASYNC_JOB_START) < 0) + QEMU_ASYNC_JOB_START, &createParams) < 0) goto endjob; dom->id = vm->def->id; @@ -15769,6 +15784,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, bool was_stopped = false; qemuDomainSaveCookiePtr cookie; virCPUDefPtr origCPU = NULL; + const virCreateParams createParams = { 0 }; virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED | @@ -15990,7 +16006,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, cookie ? cookie->cpu : NULL, QEMU_ASYNC_JOB_START, NULL, -1, NULL, snap, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, - VIR_QEMU_PROCESS_START_PAUSED); + &createParams, VIR_QEMU_PROCESS_START_PAUSED); virDomainAuditStart(vm, "from-snapshot", rc >= 0); detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT; event = virDomainEventLifecycleNewFromObj(vm, @@ -16083,7 +16099,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, qemuDomainEventQueue(driver, event); rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL, QEMU_ASYNC_JOB_START, NULL, -1, NULL, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_CREATE, + VIR_NETDEV_VPORT_PROFILE_OP_CREATE, &createParams, start_flags); virDomainAuditStart(vm, "from-snapshot", rc >= 0); if (rc < 0) { @@ -18393,7 +18409,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, qemuDomainObjPrivatePtr priv = NULL; virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; - virDomainBlockIoTuneInfo reply = {0}; + virDomainBlockIoTuneInfo reply = { 0 }; char *device = NULL; int ret = -1; int maxparams; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7602a304c57f..6709af8cba16 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2178,6 +2178,7 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, int rv; char *tlsAlias = NULL; char *secAlias = NULL; + const virCreateParams createParams = { 0 }; virNWFilterReadLockFilterUpdates(); @@ -2335,7 +2336,7 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, startFlags = VIR_QEMU_PROCESS_START_AUTODESTROY; if (qemuProcessInit(driver, vm, mig->cpu, QEMU_ASYNC_JOB_MIGRATION_IN, - true, startFlags) < 0) + true, &createParams, startFlags) < 0) goto stopjob; stopProcess = true; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 37876b8d0aa6..218eb4691a86 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5008,6 +5008,7 @@ qemuProcessInit(virQEMUDriverPtr driver, virCPUDefPtr updatedCPU, qemuDomainAsyncJob asyncJob, bool migration, + const virCreateParams *createParams, unsigned int flags) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); @@ -5061,6 +5062,9 @@ qemuProcessInit(virQEMUDriverPtr driver, if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm) < 0) goto cleanup; + if (virDomainDefOverrideBootConf(vm->def, createParams) < 0) + goto cleanup; + if (flags & VIR_QEMU_PROCESS_START_PRETEND) { if (qemuDomainSetPrivatePaths(driver, vm) < 0) { virDomainObjRemoveTransientDef(vm); @@ -6296,6 +6300,7 @@ qemuProcessStart(virConnectPtr conn, const char *migratePath, virDomainSnapshotObjPtr snapshot, virNetDevVPortProfileOp vmop, + const virCreateParams *createParams, unsigned int flags) { qemuDomainObjPrivatePtr priv = vm->privateData; @@ -6307,11 +6312,11 @@ qemuProcessStart(virConnectPtr conn, VIR_DEBUG("conn=%p driver=%p vm=%p name=%s id=%d asyncJob=%s " "migrateFrom=%s migrateFd=%d migratePath=%s " - "snapshot=%p vmop=%d flags=0x%x", + "snapshot=%p vmop=%d createParams=%p flags=0x%x", conn, driver, vm, vm->def->name, vm->def->id, qemuDomainAsyncJobTypeToString(asyncJob), NULLSTR(migrateFrom), migrateFd, NULLSTR(migratePath), - snapshot, vmop, flags); + snapshot, vmop, createParams, flags); virCheckFlagsGoto(VIR_QEMU_PROCESS_START_COLD | VIR_QEMU_PROCESS_START_PAUSED | @@ -6320,8 +6325,8 @@ qemuProcessStart(virConnectPtr conn, if (!migrateFrom && !snapshot) flags |= VIR_QEMU_PROCESS_START_NEW; - if (qemuProcessInit(driver, vm, updatedCPU, - asyncJob, !!migrateFrom, flags) < 0) + if (qemuProcessInit(driver, vm, updatedCPU, asyncJob, !!migrateFrom, + createParams, flags) < 0) goto cleanup; if (migrateFrom) { @@ -6396,6 +6401,7 @@ qemuProcessCreatePretendCmd(virQEMUDriverPtr driver, unsigned int flags) { virCommandPtr cmd = NULL; + const virCreateParams createParams = { 0 }; virCheckFlagsGoto(VIR_QEMU_PROCESS_START_COLD | VIR_QEMU_PROCESS_START_PAUSED | @@ -6405,7 +6411,7 @@ qemuProcessCreatePretendCmd(virQEMUDriverPtr driver, flags |= VIR_QEMU_PROCESS_START_NEW; if (qemuProcessInit(driver, vm, NULL, QEMU_ASYNC_JOB_NONE, - !!migrateURI, flags) < 0) + !!migrateURI, &createParams, flags) < 0) goto cleanup; if (qemuProcessPrepareDomain(driver, vm, flags) < 0) diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 9dd5c97642db..e344ad532618 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -91,6 +91,7 @@ int qemuProcessStart(virConnectPtr conn, const char *stdin_path, virDomainSnapshotObjPtr snapshot, virNetDevVPortProfileOp vmop, + const virCreateParams *params, unsigned int flags); virCommandPtr qemuProcessCreatePretendCmd(virQEMUDriverPtr driver, @@ -105,6 +106,7 @@ int qemuProcessInit(virQEMUDriverPtr driver, virCPUDefPtr updatedCPU, qemuDomainAsyncJob asyncJob, bool migration, + const virCreateParams *params, unsigned int flags); int qemuProcessPrepareDomain(virQEMUDriverPtr driver, -- 2.13.4

Add support for the API virDomainCreateWithParams to the QEMU driver. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/qemu/qemu_driver.c | 49 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9a43d4157b09..2156078feffe 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7332,24 +7332,55 @@ qemuDomainObjStart(virConnectPtr conn, static int -qemuDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) +qemuDomainCreateWithParams(virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; - const virCreateParams createParams = { 0 }; + size_t i; + virCreateParams createParams = { 0 }; virCheckFlags(VIR_DOMAIN_START_PAUSED | VIR_DOMAIN_START_AUTODESTROY | VIR_DOMAIN_START_BYPASS_CACHE | VIR_DOMAIN_START_FORCE_BOOT, -1); + if (virTypedParamsValidate(params, nparams, + VIR_DOMAIN_CREATE_PARM_DEVICE_IDENTIFIER, + VIR_TYPED_PARAM_STRING, + VIR_DOMAIN_CREATE_PARM_KERNEL, + VIR_TYPED_PARAM_STRING, + VIR_DOMAIN_CREATE_PARM_INITRD, + VIR_TYPED_PARAM_STRING, + VIR_DOMAIN_CREATE_PARM_CMDLINE, + VIR_TYPED_PARAM_STRING, + NULL) < 0) + return -1; + + for (i = 0; i < nparams; i++) { + virTypedParameterPtr param = ¶ms[i]; + const char *value_str = param->value.s; + + if (STREQ(param->field, VIR_DOMAIN_CREATE_PARM_DEVICE_IDENTIFIER)) { + createParams.bootDeviceIdentifier = value_str; + } else if (STREQ(param->field, VIR_DOMAIN_CREATE_PARM_KERNEL)) { + createParams.kernel = value_str; + } else if (STREQ(param->field, VIR_DOMAIN_CREATE_PARM_INITRD)) { + createParams.initrd = value_str; + } else if (STREQ(param->field, VIR_DOMAIN_CREATE_PARM_CMDLINE)) { + createParams.cmdline = value_str; + } + } + virNWFilterReadLockFilterUpdates(); if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - if (virDomainCreateWithFlagsEnsureACL(dom->conn, vm->def) < 0) + if (virDomainCreateWithParamsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (qemuProcessBeginJob(driver, vm, VIR_DOMAIN_JOB_OPERATION_START, @@ -7378,12 +7409,21 @@ qemuDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) return ret; } + +static int +qemuDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) +{ + return qemuDomainCreateWithParams(dom, NULL, 0, flags); +} + + static int qemuDomainCreate(virDomainPtr dom) { - return qemuDomainCreateWithFlags(dom, 0); + return qemuDomainCreateWithParams(dom, NULL, 0, 0); } + static virDomainPtr qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml, @@ -21421,6 +21461,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .connectNumOfDefinedDomains = qemuConnectNumOfDefinedDomains, /* 0.2.0 */ .domainCreate = qemuDomainCreate, /* 0.2.0 */ .domainCreateWithFlags = qemuDomainCreateWithFlags, /* 0.8.2 */ + .domainCreateWithParams = qemuDomainCreateWithParams, /* 4.4.0 */ .domainDefineXML = qemuDomainDefineXML, /* 0.2.0 */ .domainDefineXMLFlags = qemuDomainDefineXMLFlags, /* 1.2.12 */ .domainUndefine = qemuDomainUndefine, /* 0.2.0 */ -- 2.13.4

Implement virConnectSupportsFeature for the test driver as this API is used by various API functions (the functions usually use the macro VIR_DRV_SUPPORTS_FEATURE for calling virConnectSupportsFeature). The support for the feature VIR_DRV_FEATURE_TYPED_PARAM_STRING is needed for the upcoming test case for the API `virDomainCreateWithParams`. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/test/test_driver.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 467587b19fa8..2a85f87684dd 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6810,6 +6810,33 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } +static int +testConnectSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, + int feature) +{ + switch (feature) { + case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: + case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + return 1; + case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: + case VIR_DRV_FEATURE_FD_PASSING: + case VIR_DRV_FEATURE_MIGRATION_V1: + case VIR_DRV_FEATURE_REMOTE: + case VIR_DRV_FEATURE_MIGRATION_V2: + case VIR_DRV_FEATURE_MIGRATION_P2P: + case VIR_DRV_FEATURE_MIGRATION_DIRECT: + case VIR_DRV_FEATURE_MIGRATION_V3: + case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION: + case VIR_DRV_FEATURE_XML_MIGRATABLE: + case VIR_DRV_FEATURE_MIGRATION_OFFLINE: + case VIR_DRV_FEATURE_MIGRATION_PARAMS: + case VIR_DRV_FEATURE_PROGRAM_KEEPALIVE: + return 0; + } + + return 0; +} + static virHypervisorDriver testHypervisorDriver = { .name = "Test", @@ -6818,6 +6845,7 @@ static virHypervisorDriver testHypervisorDriver = { .connectGetVersion = testConnectGetVersion, /* 0.1.1 */ .connectGetHostname = testConnectGetHostname, /* 0.6.3 */ .connectGetMaxVcpus = testConnectGetMaxVcpus, /* 0.3.2 */ + .connectSupportsFeature = testConnectSupportsFeature, /* 4.4.0 */ .nodeGetInfo = testNodeGetInfo, /* 0.1.1 */ .nodeGetCPUStats = testNodeGetCPUStats, /* 2.3.0 */ .nodeGetFreeMemory = testNodeGetFreeMemory, /* 2.3.0 */ -- 2.13.4

Add support for virDomainCreateWithParams to the test driver. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/test/test_driver.c | 74 ++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 62 insertions(+), 12 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 2a85f87684dd..cb5377004df7 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -650,7 +650,8 @@ testDomainShutdownState(virDomainPtr domain, static int testDomainStartState(testDriverPtr privconn, virDomainObjPtr dom, - virDomainRunningReason reason) + virDomainRunningReason reason, + const virCreateParams *createParams) { int ret = -1; @@ -663,6 +664,9 @@ testDomainStartState(testDriverPtr privconn, goto cleanup; } + if (virDomainDefOverrideBootConf(dom->def, createParams) < 0) + goto cleanup; + dom->hasManagedSave = false; ret = 0; cleanup: @@ -930,7 +934,7 @@ testParseDomains(testDriverPtr privconn, if (nsdata->runstate != VIR_DOMAIN_SHUTOFF) { if (testDomainStartState(privconn, obj, - VIR_DOMAIN_RUNNING_BOOTED) < 0) + VIR_DOMAIN_RUNNING_BOOTED, NULL) < 0) goto error; } else { testDomainShutdownState(NULL, obj, 0); @@ -1674,7 +1678,7 @@ testDomainCreateXML(virConnectPtr conn, const char *xml, goto cleanup; def = NULL; - if (testDomainStartState(privconn, dom, VIR_DOMAIN_RUNNING_BOOTED) < 0) { + if (testDomainStartState(privconn, dom, VIR_DOMAIN_RUNNING_BOOTED, NULL) < 0) { if (!dom->persistent) virDomainObjListRemove(privconn->domains, dom); goto cleanup; @@ -2185,7 +2189,7 @@ testDomainRestoreFlags(virConnectPtr conn, goto cleanup; def = NULL; - if (testDomainStartState(privconn, dom, VIR_DOMAIN_RUNNING_RESTORED) < 0) { + if (testDomainStartState(privconn, dom, VIR_DOMAIN_RUNNING_RESTORED, NULL) < 0) { if (!dom->persistent) virDomainObjListRemove(privconn->domains, dom); goto cleanup; @@ -2959,17 +2963,50 @@ testNodeGetFreePages(virConnectPtr conn ATTRIBUTE_UNUSED, return 0; } -static int testDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) + +static int +testDomainCreateWithParams(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags) { testDriverPtr privconn = domain->conn->privateData; virDomainObjPtr privdom; virObjectEventPtr event = NULL; int ret = -1; + size_t i; + virCreateParams createParams = {0}; virCheckFlags(0, -1); - testDriverLock(privconn); + if (virTypedParamsValidate(params, nparams, + VIR_DOMAIN_CREATE_PARM_DEVICE_IDENTIFIER, + VIR_TYPED_PARAM_STRING, + VIR_DOMAIN_CREATE_PARM_KERNEL, + VIR_TYPED_PARAM_STRING, + VIR_DOMAIN_CREATE_PARM_INITRD, + VIR_TYPED_PARAM_STRING, + VIR_DOMAIN_CREATE_PARM_CMDLINE, + VIR_TYPED_PARAM_STRING, + NULL) < 0) + return -1; + for (i = 0; i < nparams; i++) { + virTypedParameterPtr param = ¶ms[i]; + const char *value_str = param->value.s; + + if (STREQ(param->field, VIR_DOMAIN_CREATE_PARM_DEVICE_IDENTIFIER)) { + createParams.bootDeviceIdentifier = value_str; + } else if (STREQ(param->field, VIR_DOMAIN_CREATE_PARM_KERNEL)) { + createParams.kernel = value_str; + } else if (STREQ(param->field, VIR_DOMAIN_CREATE_PARM_INITRD)) { + createParams.initrd = value_str; + } else if (STREQ(param->field, VIR_DOMAIN_CREATE_PARM_CMDLINE)) { + createParams.cmdline = value_str; + } + } + + testDriverLock(privconn); if (!(privdom = testDomObjFromDomain(domain))) goto cleanup; @@ -2980,13 +3017,14 @@ static int testDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) } if (testDomainStartState(privconn, privdom, - VIR_DOMAIN_RUNNING_BOOTED) < 0) + VIR_DOMAIN_RUNNING_BOOTED, &createParams) < 0) goto cleanup; + domain->id = privdom->def->id; event = virDomainEventLifecycleNewFromObj(privdom, - VIR_DOMAIN_EVENT_STARTED, - VIR_DOMAIN_EVENT_STARTED_BOOTED); + VIR_DOMAIN_EVENT_STARTED, + VIR_DOMAIN_EVENT_STARTED_BOOTED); ret = 0; cleanup: @@ -2996,11 +3034,22 @@ static int testDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) return ret; } -static int testDomainCreate(virDomainPtr domain) + +static int +testDomainCreateWithFlags(virDomainPtr domain, + unsigned int flags) { - return testDomainCreateWithFlags(domain, 0); + return testDomainCreateWithParams(domain, NULL, 0, flags); } + +static int +testDomainCreate(virDomainPtr domain) +{ + return testDomainCreateWithParams(domain, NULL, 0, 0); +} + + static int testDomainUndefineFlags(virDomainPtr domain, unsigned int flags) { @@ -6728,7 +6777,7 @@ testDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, was_stopped = true; virDomainObjAssignDef(vm, config, false, NULL); if (testDomainStartState(privconn, vm, - VIR_DOMAIN_RUNNING_FROM_SNAPSHOT) < 0) + VIR_DOMAIN_RUNNING_FROM_SNAPSHOT, NULL) < 0) goto cleanup; event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, @@ -6891,6 +6940,7 @@ static virHypervisorDriver testHypervisorDriver = { .connectNumOfDefinedDomains = testConnectNumOfDefinedDomains, /* 0.1.11 */ .domainCreate = testDomainCreate, /* 0.1.11 */ .domainCreateWithFlags = testDomainCreateWithFlags, /* 0.8.2 */ + .domainCreateWithParams = testDomainCreateWithParams, /* 4.4.0 */ .domainDefineXML = testDomainDefineXML, /* 0.1.11 */ .domainDefineXMLFlags = testDomainDefineXMLFlags, /* 1.2.12 */ .domainUndefine = testDomainUndefine, /* 0.1.11 */ -- 2.13.4

Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/test/test_driver.c | 6 + tests/objecteventtest.c | 321 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 327 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index cb5377004df7..6536e41bf4ad 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -444,6 +444,12 @@ static const char *defaultConnXML = " <os>" " <type>hvm</type>" " </os>" +" <devices>" +" <disk type='file' device='disk'>" +" <source file='/tmp/na'/>" +" <target dev='vda'/>" +" </disk>" +" </devices>" "</domain>" "" "<network>" diff --git a/tests/objecteventtest.c b/tests/objecteventtest.c index 4b12572eb45b..cc7287e2fc66 100644 --- a/tests/objecteventtest.c +++ b/tests/objecteventtest.c @@ -472,6 +472,325 @@ testDomainStartStopEvent(const void *data) return ret; } + +typedef struct { + int count_boot_order; + int count_os_boot; + char *bootdeviceIdentifier; + char *kernel; + char *initrd; + char *cmdline; +} bootConfiguration; + + +static void +bootConfigurationFree(bootConfiguration *conf) +{ + if (!conf) + return; + + VIR_FREE(conf->bootdeviceIdentifier); + VIR_FREE(conf->kernel); + VIR_FREE(conf->initrd); + VIR_FREE(conf->cmdline); + VIR_FREE(conf); +} + + +static bool +bootConfigurationEqual(bootConfiguration *a, + bootConfiguration *b) +{ + if (!a || !b) + return a == b; + + return a->count_boot_order == b->count_boot_order && + a->count_os_boot == b->count_os_boot && + STREQ_NULLABLE(a->bootdeviceIdentifier, b->bootdeviceIdentifier) && + STREQ_NULLABLE(a->kernel, b->kernel) && + STREQ_NULLABLE(a->initrd, b->initrd) && + STREQ_NULLABLE(a->cmdline, b->cmdline); +} + + +/* Caller must free() the returned value */ +static bootConfiguration* +getBootConfiguration(virDomainPtr dom) +{ + bootConfiguration* ret; + char *xml = NULL; + xmlDocPtr doc = NULL; + xmlXPathContextPtr ctxt = NULL; + xmlNodePtr node = NULL; + + if (VIR_ALLOC(ret) < 0) + return NULL; + + if (!(xml = virDomainGetXMLDesc(dom, 0))) + goto error; + + if (!(doc = virXMLParseStringCtxt(xml, "(domain_definition)", &ctxt))) + goto error; + + ret->kernel = virXPathString("string(./os/kernel[1])", ctxt); + ret->initrd = virXPathString("string(./os/initrd[1])", ctxt); + ret->cmdline = virXPathString("string(./os/cmdline[1])", ctxt); + + if (virXPathInt("count(./os/boot)", ctxt, &ret->count_boot_order) < 0) + goto error; + + if ((virXPathInt("count(./devices/*/boot[@order='1'])", ctxt, &ret->count_boot_order) < 0)) + goto error; + + if (ret->count_boot_order > 0) { + node = virXPathNode("./devices/*/boot[@order='1']/..", ctxt); + if (!node) + goto error; + + ctxt->node = node; + + /* As we're using a heuristic for setting the boot device do + * the same here. + * + * Represents the XML node a disk? */ + ret->bootdeviceIdentifier = virXPathString("string(./target/@dev)", ctxt); + + /* Represents the XML node a network interface? (we only allow + * MAC addresses as boot device identifier for the tests (at + * least for the moment)) */ + if (!ret->bootdeviceIdentifier) + ret->bootdeviceIdentifier = virXPathString("string(./mac/@address)", ctxt); + } else { + ret->bootdeviceIdentifier = NULL; + } + + cleanup: + xmlFreeDoc(doc); + xmlXPathFreeContext(ctxt); + VIR_FREE(xml); + return ret; + + error: + bootConfigurationFree(ret); + ret = NULL; + goto cleanup; +} + + +static int +verifyOriginalState(virDomainPtr dom, bootConfiguration *original_conf) +{ + bool ret; + bootConfiguration *current_conf = getBootConfiguration(dom); + + if (!current_conf) + return false; + + ret = bootConfigurationEqual(original_conf, + current_conf); + bootConfigurationFree(current_conf); + return ret; +} + + +static int +verifyChanges(virDomainPtr dom, + const char *bootdevice, + const char *kernel, + const char *initrd, + const char *cmdline) +{ + int ret = -1; + bootConfiguration *current_conf; + + if (!(current_conf = getBootConfiguration(dom))) + goto cleanup; + + /* verify the new boot order */ + if (bootdevice) { + if (STRNEQ_NULLABLE(current_conf->bootdeviceIdentifier, bootdevice)) + goto cleanup; + + if (current_conf->count_os_boot != 0) + goto cleanup; + + if (current_conf->count_boot_order < 1) + goto cleanup; + } + + /* verify the other OS node changes */ + if ((kernel && virStringIsEmpty(kernel) && current_conf->kernel) || + (!virStringIsEmpty(kernel) && STRNEQ_NULLABLE(current_conf->kernel, kernel))) + goto cleanup; + + if ((initrd && virStringIsEmpty(initrd) && current_conf->initrd) || + (!virStringIsEmpty(initrd) && STRNEQ_NULLABLE(current_conf->initrd, initrd))) + goto cleanup; + + if ((cmdline && virStringIsEmpty(cmdline) && current_conf->cmdline) || + (!virStringIsEmpty(cmdline) && STRNEQ_NULLABLE(current_conf->cmdline, cmdline))) + goto cleanup; + + ret = 0; + cleanup: + bootConfigurationFree(current_conf); + return ret; +} + + +static int +testDomainCreateWithParamsHelper(virDomainPtr dom, lifecycleEventCounter *counter, + bool failure_expected, const char *bootdevice, + const char *kernel, const char *initrd, + const char *cmdline, unsigned int flags, bootConfiguration *original_conf) +{ + int rc; + int ret = -1; + virTypedParameterPtr params = NULL; + int nparams = 0; + int maxparams = 0; + + lifecycleEventCounter_reset(counter); + + if (bootdevice) + virTypedParamsAddFromString(¶ms, + &nparams, + &maxparams, + VIR_DOMAIN_CREATE_PARM_DEVICE_IDENTIFIER, + VIR_TYPED_PARAM_STRING, + bootdevice); + + if (kernel) + virTypedParamsAddFromString(¶ms, + &nparams, + &maxparams, + VIR_DOMAIN_CREATE_PARM_KERNEL, + VIR_TYPED_PARAM_STRING, + kernel); + + if (initrd) + virTypedParamsAddFromString(¶ms, + &nparams, + &maxparams, + VIR_DOMAIN_CREATE_PARM_INITRD, + VIR_TYPED_PARAM_STRING, + initrd); + + if (cmdline) + virTypedParamsAddFromString(¶ms, + &nparams, + &maxparams, + VIR_DOMAIN_CREATE_PARM_CMDLINE, + VIR_TYPED_PARAM_STRING, + cmdline); + + rc = virDomainCreateWithParams(dom, + params, + nparams, + flags); + if (rc < 0) { + if (failure_expected) + ret = 0; + goto cleanup; + } + + if (virEventRunDefaultImpl() < 0) + goto cleanup; + + if (counter->startEvents != 1 || + counter->stopEvents != 0) + goto cleanup; + + if (verifyChanges(dom, bootdevice, kernel, initrd, cmdline) < 0) + goto cleanup; + + if (virDomainDestroy(dom) < 0) + goto cleanup; + + if (verifyOriginalState(dom, original_conf) < 0) + goto cleanup; + + if (virEventRunDefaultImpl() < 0) + goto cleanup; + + if (counter->startEvents != 1 || + counter->stopEvents != 1) + goto cleanup; + + ret = 0; + cleanup: + virTypedParamsFree(params, nparams); + return ret; +} + + +static int +testDomainCreateWithParams(const void *data) +{ + const objecteventTest *test = data; + lifecycleEventCounter counter; + int eventId = VIR_DOMAIN_EVENT_ID_LIFECYCLE; + int id; + int ret = -1; + virDomainPtr dom; + bootConfiguration *original_boot_conf = NULL; + + dom = virDomainLookupByName(test->conn, "test"); + if (!dom) + return -1; + + /* First clean up, register for the life cycle events, and get the + * original, persistent boot configuration of the domain */ + virDomainDestroy(dom); + + id = virConnectDomainEventRegisterAny(test->conn, dom, eventId, + VIR_DOMAIN_EVENT_CALLBACK(&domainLifecycleCb), + &counter, NULL); + + if (!(original_boot_conf = getBootConfiguration(dom))) + goto cleanup; + + if (testDomainCreateWithParamsHelper(dom, &counter, true, "notAvailableBootDevice", + NULL, NULL, NULL, 0, original_boot_conf) < 0) + goto cleanup; + if (testDomainCreateWithParamsHelper(dom, &counter, false, NULL, NULL, NULL, + NULL, 0, original_boot_conf) < 0) + goto cleanup; + if (testDomainCreateWithParamsHelper(dom, &counter, false, NULL, "newKernel", + NULL, NULL, 0, original_boot_conf) < 0) + goto cleanup; + if (testDomainCreateWithParamsHelper(dom, &counter, false, NULL, NULL, "newInitrd", + NULL, 0, original_boot_conf) < 0) + goto cleanup; + if (testDomainCreateWithParamsHelper(dom, &counter, true, "notAvailableBootDevice", + "newInitrd", NULL, NULL, 0, original_boot_conf) < 0) + goto cleanup; + if (testDomainCreateWithParamsHelper(dom, &counter, false, NULL, NULL, NULL, "newCmdline", + 0, original_boot_conf) < 0) + goto cleanup; + if (testDomainCreateWithParamsHelper(dom, &counter, false, NULL, "newKernel", "newInitrd", + "newCmdline", 0, original_boot_conf) < 0) + goto cleanup; + if (testDomainCreateWithParamsHelper(dom, &counter, false, NULL, "", "", "", 0, + original_boot_conf) < 0) + goto cleanup; + if (testDomainCreateWithParamsHelper(dom, &counter, false, "vda", NULL, NULL, NULL, + 0, original_boot_conf) < 0) + goto cleanup; + if (testDomainCreateWithParamsHelper(dom, &counter, false, "vda", NULL, "blaa", "bla", + 0, original_boot_conf) < 0) + goto cleanup; + + ret = 0; + cleanup: + bootConfigurationFree(original_boot_conf); + virConnectDomainEventDeregisterAny(test->conn, id); + virDomainFree(dom); + + return ret; +} + + static int testNetworkCreateXML(const void *data) { @@ -864,6 +1183,8 @@ mymain(void) ret = EXIT_FAILURE; if (virTestRun("Domain start stop events", testDomainStartStopEvent, &test) < 0) ret = EXIT_FAILURE; + if (virTestRun("Domain start stop events with params", testDomainCreateWithParams, &test) < 0) + ret = EXIT_FAILURE; /* Network event tests */ /* Tests requiring the test network not to be set up*/ -- 2.13.4

Add the options with-bootdevice, with-kernel, with-initrd, and with-cmdline to 'virsh start'. They allow to temporarily boot from another boot device, to use another kernel, initrd, and cmdline than defined in the persistent domain definition. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- tools/virsh-domain.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 14 +++++++++ 2 files changed, 98 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 7cf8373f05bc..3cb597b90937 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3953,6 +3953,25 @@ static const vshCmdOptDef opts_start[] = { .type = VSH_OT_STRING, .help = N_("pass file descriptors N,M,... to the guest") }, + {.name = "with-bootdevice", + .type = VSH_OT_STRING, + .help = N_("set boot device") + }, + {.name = "with-kernel", + .type = VSH_OT_STRING, + .flags = VSH_OFLAG_EMPTY_OK, + .help = N_("set boot kernel") + }, + {.name = "with-initrd", + .type = VSH_OT_STRING, + .flags = VSH_OFLAG_EMPTY_OK, + .help = N_("set boot initrd") + }, + {.name = "with-cmdline", + .type = VSH_OT_STRING, + .flags = VSH_OFLAG_EMPTY_OK, + .help = N_("set boot cmdline") + }, {.name = NULL} }; @@ -4004,6 +4023,7 @@ cmdStartGetFDs(vshControl *ctl, return -1; } + static bool cmdStart(vshControl *ctl, const vshCmd *cmd) { @@ -4016,6 +4036,10 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) int rc; size_t nfds = 0; int *fds = NULL; + const char *bootDeviceIdentifier = NULL; + const char *kernel = NULL; + const char *initrd = NULL; + const char *cmdline = NULL; if (!(dom = virshCommandOptDomainBy(ctl, cmd, NULL, VIRSH_BYNAME | VIRSH_BYUUID))) @@ -4038,9 +4062,68 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "force-boot")) flags |= VIR_DOMAIN_START_FORCE_BOOT; + if (vshCommandOptStringReq(ctl, cmd, "with-bootdevice", + &bootDeviceIdentifier) < 0 || + vshCommandOptStringReq(ctl, cmd, "with-kernel", + &kernel) < 0 || + vshCommandOptStringReq(ctl, cmd, "with-initrd", + &initrd) < 0 || + vshCommandOptStringReq(ctl, cmd, "with-cmdline", + &cmdline) < 0) + goto cleanup; + + if (nfds && (bootDeviceIdentifier || kernel || initrd || cmdline)) { + vshError(ctl, + _("Passing file descriptors together with temporarily changing" + " the boot configuration is currently not supported.")); + goto cleanup; + } + /* Prefer older API unless we have to pass extra parameters */ if (nfds) { rc = virDomainCreateWithFiles(dom, nfds, fds, flags); + } else if (bootDeviceIdentifier || kernel || initrd || cmdline) { + virTypedParameterPtr params = NULL; + int nparams = 0; + int maxparams = 0; + + if (bootDeviceIdentifier) + virTypedParamsAddFromString(¶ms, + &nparams, + &maxparams, + VIR_DOMAIN_CREATE_PARM_DEVICE_IDENTIFIER, + VIR_TYPED_PARAM_STRING, + bootDeviceIdentifier); + + if (kernel) + virTypedParamsAddFromString(¶ms, + &nparams, + &maxparams, + VIR_DOMAIN_CREATE_PARM_KERNEL, + VIR_TYPED_PARAM_STRING, + kernel); + + if (initrd) + virTypedParamsAddFromString(¶ms, + &nparams, + &maxparams, + VIR_DOMAIN_CREATE_PARM_INITRD, + VIR_TYPED_PARAM_STRING, + initrd); + + if (cmdline) + virTypedParamsAddFromString(¶ms, + &nparams, + &maxparams, + VIR_DOMAIN_CREATE_PARM_CMDLINE, + VIR_TYPED_PARAM_STRING, + cmdline); + + rc = virDomainCreateWithParams(dom, + params, + nparams, + flags); + virTypedParamsFree(params, nparams); } else if (flags) { rc = virDomainCreateWithFlags(dom, flags); /* We can emulate force boot, even for older servers that @@ -4091,6 +4174,7 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) return ret; } + /* * "save" command */ diff --git a/tools/virsh.pod b/tools/virsh.pod index 929958a9533c..54316aa33b97 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2655,6 +2655,8 @@ repeat the command. =item B<start> I<domain-name-or-uuid> [I<--console>] [I<--paused>] [I<--autodestroy>] [I<--bypass-cache>] [I<--force-boot>] [I<--pass-fds N,M,...>] +[I<--with-bootdevice DEVICE-IDENTIFIER>] [I<--with-kernel KERNEL>] +[I<--with-initrd INITRD>] [I<--with-cmdline CMDLINE>] Start a (previously defined) inactive domain, either from the last B<managedsave> state, or via a fresh boot if no managedsave state is @@ -2668,6 +2670,18 @@ the restore will avoid the file system cache, although this may slow down the operation. If I<--force-boot> is specified, then any managedsave state is discarded and a fresh boot occurs. +If the I<--with-bootdevice> option is used and supported by the driver +then the domain will start with the specified device overriding the +persistent defined boot order for this start only. Valid block devices +for this option are displayed using B<domblklist>. To specify a block +device the displayed I<Target> value has to be used. Valid network +devices are displayed using B<domiflist>. To specify a network device +the displayed I<MAC> value has to be used. + +With I<--with-kernel>, I<--with-initrd>, and I<--with-cmdline> you can +specify the kernel, initrd, and cmdline to use for this start if the +used driver supports this. + If I<--pass-fds> is specified, the argument is a comma separated list of open file descriptors which should be pass on into the guest. The file descriptors will be re-numbered in the guest, starting from 3. This -- 2.13.4

Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 80181415c635..06693985839a 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -35,6 +35,17 @@ <libvirt> <release version="v4.4.0" date="unreleased"> <section title="New features"> + <change> + <summary> + Add virDomainCreateWithParams API + </summary> + <description> + Provide a new API to allow to temporarily boot from another + boot device, to use another kernel, initrd, and cmdline than + defined in the persistent domain definition. It's + implemented for the QEMU, remote, and test driver. + </description> + </change> </section> <section title="Improvements"> </section> -- 2.13.4
participants (6)
-
Christian Borntraeger
-
Daniel P. Berrangé
-
John Ferlan
-
Marc Hartmayer
-
Marc Hartmayer
-
Pino Toscano