All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: Andrew Melnychenko <andrew@daynix.com>,
	mst@redhat.com, jasowang@redhat.com, qemu-devel@nongnu.org,
	armbru@redhat.com, yuri.benditovich@daynix.com, yan@daynix.com
Subject: Re: [RFC PATCH 4/5] qmp: Added qemu-ebpf-rss-path command.
Date: Fri, 11 Jun 2021 18:21:55 +0100	[thread overview]
Message-ID: <YMObs8hO4FTgXy91@redhat.com> (raw)
In-Reply-To: <20210611141552.ezkybuvewffln4dz@redhat.com>

On Fri, Jun 11, 2021 at 09:15:52AM -0500, Eric Blake wrote:
> On Wed, Jun 09, 2021 at 01:04:56PM +0300, Andrew Melnychenko wrote:
> > New qmp command to query ebpf helper.
> > It's crucial that qemu and helper are in sync and in touch.
> > Technically helper should pass eBPF fds that qemu may accept.
> > And different qemu's builds may have different eBPF programs and helpers.
> > Qemu returns helper that should "fit" to virtio-net.
> > 
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> > ---
> >  monitor/qmp-cmds.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++
> >  qapi/misc.json     | 29 +++++++++++++++++
> >  2 files changed, 107 insertions(+)
> > 
> > diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> > index f7d64a6457..5dd2a58ea2 100644
> > --- a/monitor/qmp-cmds.c
> > +++ b/monitor/qmp-cmds.c
> > @@ -351,3 +351,81 @@ void qmp_display_reload(DisplayReloadOptions *arg, Error **errp)
> >          abort();
> >      }
> >  }
> > +
> > +#ifdef CONFIG_LINUX
> > +
> > +static const char *get_dirname(char *path)
> > +{
> > +    char *sep;
> > +
> > +    sep = strrchr(path, '/');
> > +    if (sep == path) {
> > +        return "/";
> > +    } else if (sep) {
> > +        *sep = 0;
> > +        return path;
> > +    }
> > +    return ".";
> > +}
> 
> Seems like this function is duplicating what glib should already be
> able to do.
> 
> > +
> > +static char *find_helper(const char *name)
> > +{
> > +    char qemu_exec[PATH_MAX];
> 
> Stack-allocating a PATH_MAX array for readlink() is poor practice.
> Better is to use g_file_read_link().
> 
> > +    const char *qemu_dir = NULL;
> > +    char *helper = NULL;
> > +
> > +    if (name == NULL) {
> > +        return NULL;
> > +    }
> > +
> > +    if (readlink("/proc/self/exe", qemu_exec, PATH_MAX) > 0) {
> > +        qemu_dir = get_dirname(qemu_exec);
> > +
> > +        helper = g_strdup_printf("%s/%s", qemu_dir, name);
> > +        if (access(helper, F_OK) == 0) {
> > +            return helper;
> > +        }
> > +        g_free(helper);
> > +    }
> > +
> > +    helper = g_strdup_printf("%s/%s", CONFIG_QEMU_HELPERDIR, name);
> 
> Could we use a compile-time determination of where we were (supposed)
> to be installed, and therefore where our helper should be installed,
> rather than the dynamic /proc/self/exe munging?

Yeah I think avoiding /proc/self/exe is desirable, because I can
imagine scenarios where this can lead to picking the wrong helper.
Better to always use the compile time install directory.


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:[~2021-06-11 17:24 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-09 10:04 [RFC PATCH 0/5] ebpf: Added ebpf helper for libvirtd Andrew Melnychenko
2021-06-09 10:04 ` [RFC PATCH 1/5] ebpf: Added eBPF initialization by fds Andrew Melnychenko
2021-06-09 10:04 ` [RFC PATCH 2/5] virtio-net: Added property to load eBPF RSS with fds Andrew Melnychenko
2021-06-09 10:04 ` [RFC PATCH 3/5] ebpf_rss_helper: Added helper for eBPF RSS Andrew Melnychenko
2021-06-09 10:04 ` [RFC PATCH 4/5] qmp: Added qemu-ebpf-rss-path command Andrew Melnychenko
2021-06-11 14:15   ` Eric Blake
2021-06-11 17:21     ` Daniel P. Berrangé [this message]
2021-06-12  5:28   ` Markus Armbruster
2021-06-15 23:16     ` Andrew Melnichenko
2021-07-05 13:50       ` Andrew Melnichenko
2021-06-09 10:04 ` [RFC PATCH 5/5] meson: libbpf dependency now exclusively for Linux Andrew Melnychenko
2021-06-10  6:41 ` [RFC PATCH 0/5] ebpf: Added ebpf helper for libvirtd Jason Wang
2021-06-10  6:55   ` Yuri Benditovich
2021-06-11  5:36     ` Jason Wang
2021-06-11 16:49       ` Andrew Melnichenko
2021-06-11 17:24         ` Daniel P. Berrangé
2021-06-15  9:13         ` Jason Wang
2021-06-15 22:18           ` Andrew Melnichenko
2021-06-18 20:03             ` Andrew Melnichenko
2021-06-21  9:20               ` Jason Wang
2021-06-22  3:29                 ` Yuri Benditovich
2021-06-22  4:58                   ` Jason Wang
2021-06-22  8:25                     ` Toke Høiland-Jørgensen
2021-06-22  8:27                       ` Daniel P. Berrangé
2021-06-22  9:09                         ` Toke Høiland-Jørgensen
2021-06-22 13:01                           ` Andrew Melnichenko
2021-06-22 13:17                             ` Toke Høiland-Jørgensen
2021-06-23  0:47                           ` Jason Wang
2021-06-28 11:18                             ` Yuri Benditovich
2021-06-29  3:39                               ` Jason Wang
2021-06-30 16:40                                 ` Andrew Melnichenko

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=YMObs8hO4FTgXy91@redhat.com \
    --to=berrange@redhat.com \
    --cc=andrew@daynix.com \
    --cc=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=yan@daynix.com \
    --cc=yuri.benditovich@daynix.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.