qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: John Snow <jsnow@redhat.com>
Cc: qemu-devel@nongnu.org, "Cédric Le Goater" <clg@kaod.org>,
	"Hanna Reitz" <hreitz@redhat.com>,
	"Ani Sinha" <anisinha@redhat.com>,
	"Cleber Rosa" <crosa@redhat.com>,
	"Wainer dos Santos Moschetta" <wainersm@redhat.com>,
	qemu-arm@nongnu.org, qemu-block@nongnu.org,
	"Andrew Jeffery" <andrew@aj.id.au>,
	"Joel Stanley" <joel@jms.id.au>, "Kevin Wolf" <kwolf@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Beraldo Leal" <bleal@redhat.com>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [PATCH v2 6/6] python/machine: remove unused sock_dir argument
Date: Tue, 25 Jul 2023 19:36:35 +0100	[thread overview]
Message-ID: <ZMAWM7BVs5gLwiUl@redhat.com> (raw)
In-Reply-To: <CAFn=p-YTohvmxET2kk1NWU1X=psVcD1M0oS7BDEZUH521DEQ_w@mail.gmail.com>

On Tue, Jul 25, 2023 at 02:33:36PM -0400, John Snow wrote:
> On Tue, Jul 25, 2023 at 2:26 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Tue, Jul 25, 2023 at 02:03:37PM -0400, John Snow wrote:
> > > By using a socketpair for all of the sockets managed by the VM class and
> > > its extensions, we don't need the sock_dir argument anymore, so remove
> > > it.
> > >
> > > We only added this argument so that we could specify a second, shorter
> > > temporary directory for cases where the temp/log dirs were "too long" as
> > > a socket name on macOS. We don't need it for this class now. In one
> > > case, avocado testing takes over responsibility for creating an
> > > appropriate sockdir.
> > >
> > > Signed-off-by: John Snow <jsnow@redhat.com>
> > > ---
> > >  python/qemu/machine/machine.py             | 18 ------------------
> > >  python/qemu/machine/qtest.py               |  5 +----
> > >  tests/avocado/acpi-bits.py                 |  5 +----
> > >  tests/avocado/avocado_qemu/__init__.py     |  2 +-
> > >  tests/avocado/machine_aspeed.py            |  5 ++++-
> > >  tests/qemu-iotests/iotests.py              |  2 +-
> > >  tests/qemu-iotests/tests/copy-before-write |  3 +--
> > >  7 files changed, 9 insertions(+), 31 deletions(-)
> >
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> >
> 
> Thanks!
> 
> I don't know if we want this for *this* cycle or not, it's "only
> testing code" and it should hopefully be harmless. If it makes the
> tests more reliable, it might be worth it. I don't have strong
> feelings one way or the other, we've lived without it for so long
> as-is.
> 
> I'll see what Peter says.

Although it does affect end users, the biggest impact is our own
CI. Once the release is out it would help CI on stable trees,
but the big win is CI on master.

I'd verge towards skipping this during freeze and applying to
master after release. Then propose cherry-pick to stable once
it has had some soak time in our real CI.


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 :|



  reply	other threads:[~2023-07-25 18:37 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-25 18:03 [PATCH v2 0/6] python/machine: use socketpair() for console socket John Snow
2023-07-25 18:03 ` [PATCH v2 1/6] python/machine: move socket setup out of _base_args property John Snow
2023-07-25 18:16   ` Daniel P. Berrangé
2023-07-26  7:10   ` Ani Sinha
2023-07-25 18:03 ` [PATCH v2 2/6] python/machine: close sock_pair in cleanup path John Snow
2023-07-25 18:18   ` Daniel P. Berrangé
2023-07-26  7:23     ` Ani Sinha
2023-07-25 18:03 ` [PATCH v2 3/6] python/console_socket: accept existing FD in initializer John Snow
2023-07-25 18:19   ` Daniel P. Berrangé
2023-07-27  5:41   ` Ani Sinha
2023-07-25 18:03 ` [PATCH v2 4/6] python/machine: use socketpair() for console connections John Snow
2023-07-25 18:21   ` Daniel P. Berrangé
2023-07-26 10:50   ` Ani Sinha
2023-07-26 17:21     ` John Snow
2023-07-27  5:52       ` Ani Sinha
2023-07-27  6:49         ` Ani Sinha
2023-07-27 10:41         ` Daniel P. Berrangé
2023-07-25 18:03 ` [PATCH v2 5/6] python/machine: use socketpair() for qtest connection John Snow
2023-07-25 18:24   ` Daniel P. Berrangé
2023-07-25 18:03 ` [PATCH v2 6/6] python/machine: remove unused sock_dir argument John Snow
2023-07-25 18:26   ` Daniel P. Berrangé
2023-07-25 18:33     ` John Snow
2023-07-25 18:36       ` Daniel P. Berrangé [this message]
2023-07-27 13:05 ` [PATCH v2 0/6] python/machine: use socketpair() for console socket Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZMAWM7BVs5gLwiUl@redhat.com \
    --to=berrange@redhat.com \
    --cc=andrew@aj.id.au \
    --cc=anisinha@redhat.com \
    --cc=bleal@redhat.com \
    --cc=clg@kaod.org \
    --cc=crosa@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=joel@jms.id.au \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mst@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=wainersm@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).