On Thu, Jan 30, 2025 at 01:16:42PM +0100, Peter Krempa wrote:
> On Wed, Jan 08, 2025 at 19:42:39 +0000, Daniel P. Berrangé wrote:
> > This allows a user specified delay between autostart of each VM, giving
> > parity with the equivalent feature of libvirt-guests.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> > ---
> > src/qemu/libvirtd_qemu.aug | 1 +
> > src/qemu/qemu.conf.in | 4 ++++
> > src/qemu/qemu_conf.c | 2 ++
> > src/qemu/qemu_conf.h | 1 +
> > src/qemu/qemu_driver.c | 1 +
> > src/qemu/test_libvirtd_qemu.aug.in | 1 +
> > 6 files changed, 10 insertions(+)
>
> [...]
>
> > diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in
> > index d853136f10..a3e9bbfcf3 100644
> > --- a/src/qemu/qemu.conf.in
> > +++ b/src/qemu/qemu.conf.in
> > @@ -634,6 +634,10 @@
> > #
> > #auto_start_bypass_cache = 0
> >
> > +# Delay in milliseconds between starting each VM, during autostart
>
> I'd suggest you mention that the delay is between kicking off the
> startup of the autostarted VMs, so that it's obvious that we don't/can't
> see when the VM actually booted (whatever the definition of 'booted'
> would be for users).
Yeah, good idea.
Side note, for modern Linux guests users can now seen when a VM
has fully booted if they add a virtio-vsock device, as systemd
is able to notify the host over that. Not something we can/should
rely on here though.
>
>
> > +#
> > +#auto_start_delay = 0
> > +
> > # If provided by the host and a hugetlbfs mount point is configured,
> > # a guest may request huge page backing. When this mount point is
> > # unspecified here, determination of a host mount point in /proc/mounts
> > diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> > index 8b9fe4e381..0b6b923bcb 100644
> > --- a/src/qemu/qemu_conf.c
> > +++ b/src/qemu/qemu_conf.c
> > @@ -638,6 +638,8 @@ virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg,
> > return -1;
> > if (virConfGetValueBool(conf, "auto_start_bypass_cache", &cfg->autoStartBypassCache) < 0)
> > return -1;
> > + if (virConfGetValueInt(conf, "auto_start_delay", &cfg->autoStartDelayMS) < 0)
> > + return -1;
> >
> > return 0;
> > }
> > diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> > index 42cdb6f883..61a2bdce51 100644
> > --- a/src/qemu/qemu_conf.h
> > +++ b/src/qemu/qemu_conf.h
> > @@ -200,6 +200,7 @@ struct _virQEMUDriverConfig {
> > char *autoDumpPath;
> > bool autoDumpBypassCache;
> > bool autoStartBypassCache;
> > + int autoStartDelayMS;
>
> Once again, preferrably declare this as unsigned. Especially prevent
> users passing negative values.
>
> Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx>
>
With 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 :|
