All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Straub <lukasstraub2@web.de>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Thomas Huth <thuth@redhat.com>,
	Alex Bennee <alex.bennee@linaro.org>,
	qemu-devel <qemu-devel@nongnu.org>,
	Markus Armbruster <armbru@redhat.com>
Subject: Re: [PATCH 0/2] yank: Always link full yank code
Date: Wed, 24 Mar 2021 13:09:11 +0100	[thread overview]
Message-ID: <20210324130912.0490afb8@gecko.fritz.box> (raw)
In-Reply-To: <YFskLauf5ZZ4RJwi@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1879 bytes --]

On Wed, 24 Mar 2021 11:36:13 +0000
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Wed, Mar 24, 2021 at 12:22:42PM +0100, Lukas Straub wrote:
> > On Tue, 23 Mar 2021 19:09:15 +0000
> > Daniel P. Berrangé <berrange@redhat.com> wrote:
> >   
> > > On Tue, Mar 23, 2021 at 06:52:19PM +0100, Lukas Straub wrote:  
> > > > Hello Everyone,
> > > > These patches remove yank's dependency on qiochannel and always link it in.
> > > > Please Review.    
> > > 
> > > It would be useful if the cover letter or commit messages explained
> > > to potential reviewers why this is being changed... The first patch
> > > feels like a regression to me, without seeing an explanation why
> > > it is desirable.  
> > 
> > Yes, sorry. There are two reasons for this patchset:
> > -To exercise the full yank code (with checks for registering and unregistering
> >  of yank functions and instances) in existing tests and in the new yank test
> >  (https://lore.kernel.org/qemu-devel/cover.1616521487.git.lukasstraub2@web.de/)
> > -To replace "[PATCH] yank: Avoid linking into executables that don't want it"
> >  (https://lore.kernel.org/qemu-devel/20210316135907.3646901-1-armbru@redhat.com/)
> >  Now we always link in yank, but without pulling in other dependencies.  
> 
> Conceptually, the real world usage of yank is in combination with parts
> of QEMU doing I/O, and so they will always have the "io" subsystem
> available. Thus it feels wrong to be refactoring this to avoid an
> "io" dependancy.  I think this probably suggests that the yank code
> shouldn't be in libqemuutil.la, but instead part of the "io" subsystem
> to make the association/dependency explicit

Yes, makes sense. On the other hand I think that the yank functions should be
separate from the yank core anyway.

Regards,
Lukas Straub

> 
> Regards,
> Daniel



-- 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

      reply	other threads:[~2021-03-24 12:11 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-23 17:52 [PATCH 0/2] yank: Always link full yank code Lukas Straub
2021-03-23 17:52 ` [PATCH 1/2] yank: Remove dependency on qiochannel Lukas Straub
2021-03-23 18:03   ` Thomas Huth
2021-03-25 20:29   ` Marc-André Lureau
2021-03-23 17:52 ` [PATCH 2/2] yank: Always link full yank code Lukas Straub
2021-03-23 18:04   ` Thomas Huth
2021-03-25 20:30   ` Marc-André Lureau
2021-03-23 19:09 ` [PATCH 0/2] " Daniel P. Berrangé
2021-03-24 11:22   ` Lukas Straub
2021-03-24 11:36     ` Daniel P. Berrangé
2021-03-24 12:09       ` Lukas Straub [this message]

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=20210324130912.0490afb8@gecko.fritz.box \
    --to=lukasstraub2@web.de \
    --cc=alex.bennee@linaro.org \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.