Re: [libvirt PATCH v2 28/44] Deprecate QEMU_CAPS_NO_SHUTDOWN

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2018-04-09 at 17:20 +0200, Ján Tomko wrote:
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index c6cfd05216..d175b196ea 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -7486,17 +7486,10 @@ qemuProcessReconnect(void *opaque)
>          /* We can't get the monitor back, so must kill the VM
>           * to remove danger of it ending up running twice if
>           * user tries to start it again later
> -         */
> -        if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) {
> -            /* If we couldn't get the monitor and qemu supports
> -             * no-shutdown, we can safely say that the domain
> -             * crashed ... */
> -            state = VIR_DOMAIN_SHUTOFF_CRASHED;
> -        } else {
> -            /* ... but if it doesn't we can't say what the state
> -             * really is and FAILED means "failed to start" */
> -            state = VIR_DOMAIN_SHUTOFF_UNKNOWN;
> -        }
> +         * If we couldn't get the monitor and qemu supports
> +         * no-shutdown, we can safely say that the domain
> +         * crashed ... */

s/ and qemu/, since QEMU/

[...]
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index d451626be5..07f7630655 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1681,7 +1681,7 @@ mymain(void)
>              QEMU_CAPS_SCSI_LSI);
>  
>      DO_TEST("monitor-json", NONE);
> -    DO_TEST("no-shutdown", QEMU_CAPS_NO_SHUTDOWN);
> +    DO_TEST("no-shutdown", NONE);

I wonder if you could just drop the test case altogether, since
basically the whole test suite is using the feature now...

The same is actually true for the monitor-json test right above it
and probably a bunch more tests that were strictly connected to
capabilities you dropped or are going to drop. I should have
thought about that earlier, but it didn't really occur to me until
now and the series has already been partially pushed; I guess it's
a good idea to be more aggressive with the cleanup from this point
on, though. We can come back later and see whether we missed any
up until this point.

With the comment updated and the test case dropped,

  Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx>

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux