Mailing List Archive

On Wed, Apr 10, 2024 at 02:43:13PM +0300, Manos Pitsidianakis wrote:
> This patch series proposes converting the compile-time define
> LIBXL_DEVICE_MODEL_START_TIMEOUT value to an optionally overridden by
> environment variable value, just like the current behavior for
> LIBXL_BOOTLOADER_TIMEOUT is.
>
> Manos Pitsidianakis (2):
> libs/light: add device model start timeout env var
> libs/light: expand device model start timeout use
>
> docs/man/xl.1.pod.in | 11 +++++++++++
> tools/libs/light/libxl_9pfs.c | 2 +-
> tools/libs/light/libxl_device.c | 2 +-
> tools/libs/light/libxl_dm.c | 10 +++++-----
> tools/libs/light/libxl_dom_suspend.c | 2 +-
> tools/libs/light/libxl_domain.c | 5 +++--
> tools/libs/light/libxl_internal.h | 6 ++++++
> tools/libs/light/libxl_pci.c | 10 +++++-----
> tools/libs/light/libxl_usb.c | 8 ++++----
> 9 files changed, 37 insertions(+), 19 deletions(-)
>
>
> base-commit: f48299cad5c3c69fdc2c101517a6dab9c9827ea5
> --
> ???? ???? ???????
>
>

Hi Manos,

Did you know that you could run something like
`git send-email --cc-cmd="scripts/get_maintainer.pl --no-l" ...`
and git would CC the maintainers of the code?

I've configure my xen repo to have that been automatic, with
git config sendemail.cccmd 'cd `git rev-parse --show-toplevel` && scripts/get_maintainer.pl --no-l'

There's other way to send patch, like using
"scripts/add_maintainers.pl" described on this page:
https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Sending_a_Patch_Series
Not sure which one is better.

Anyway, I've added the series to my list of patch to look at. But I
might miss it next time if I'm not CCed.

Cheers,

--
Anthony PERARD

On Wed, 10 Apr 2024 at 18:53, Anthony PERARD <anthony.perard@citrix.com> wrote:
>
> On Wed, Apr 10, 2024 at 02:43:13PM +0300, Manos Pitsidianakis wrote:
> > This patch series proposes converting the compile-time define
> > LIBXL_DEVICE_MODEL_START_TIMEOUT value to an optionally overridden by
> > environment variable value, just like the current behavior for
> > LIBXL_BOOTLOADER_TIMEOUT is.
> >
> > Manos Pitsidianakis (2):
> > libs/light: add device model start timeout env var
> > libs/light: expand device model start timeout use
> >
> > docs/man/xl.1.pod.in | 11 +++++++++++
> > tools/libs/light/libxl_9pfs.c | 2 +-
> > tools/libs/light/libxl_device.c | 2 +-
> > tools/libs/light/libxl_dm.c | 10 +++++-----
> > tools/libs/light/libxl_dom_suspend.c | 2 +-
> > tools/libs/light/libxl_domain.c | 5 +++--
> > tools/libs/light/libxl_internal.h | 6 ++++++
> > tools/libs/light/libxl_pci.c | 10 +++++-----
> > tools/libs/light/libxl_usb.c | 8 ++++----
> > 9 files changed, 37 insertions(+), 19 deletions(-)
> >
> >
> > base-commit: f48299cad5c3c69fdc2c101517a6dab9c9827ea5
> > --
> > ???? ???? ???????
> >
> >
>
> Hi Manos,
>
> Did you know that you could run something like
> `git send-email --cc-cmd="scripts/get_maintainer.pl --no-l" ...`
> and git would CC the maintainers of the code?
>
> I've configure my xen repo to have that been automatic, with
> git config sendemail.cccmd 'cd `git rev-parse --show-toplevel` && scripts/get_maintainer.pl --no-l'
>
> There's other way to send patch, like using
> "scripts/add_maintainers.pl" described on this page:
> https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Sending_a_Patch_Series
> Not sure which one is better.
>
> Anyway, I've added the series to my list of patch to look at. But I
> might miss it next time if I'm not CCed.
>
> Cheers,
>
> --
> Anthony PERARD

Hello Anthony,

I do know about that, in fact I used it and it did not output your
email. Here's what the `get_maintainer.pl` outputs: (same output for
all patches):

$ ./scripts/get_maintainer.pl --no-l
../../patches/libxl-dm-timeout-v1/v1-0001-libs-light-add-device-model-start-timeout-env-var.patch
Andrew Cooper <andrew.cooper3@citrix.com>
George Dunlap <george.dunlap@citrix.com>
Jan Beulich <jbeulich@suse.com>
Julien Grall <julien@xen.org>
Stefano Stabellini <sstabellini@kernel.org>

Thanks,

--
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd

On Thu, Apr 11, 2024 at 01:23:07PM +0300, Manos Pitsidianakis wrote:
> Hello Anthony,
>
> I do know about that, in fact I used it and it did not output your
> email. Here's what the `get_maintainer.pl` outputs: (same output for
> all patches):
>
> $ ./scripts/get_maintainer.pl --no-l
> ../../patches/libxl-dm-timeout-v1/v1-0001-libs-light-add-device-model-start-timeout-env-var.patch
> Andrew Cooper <andrew.cooper3@citrix.com>
> George Dunlap <george.dunlap@citrix.com>
> Jan Beulich <jbeulich@suse.com>
> Julien Grall <julien@xen.org>
> Stefano Stabellini <sstabellini@kernel.org>

Oh, sorry, I didn't realise this was "THE REST" that is the list of
default maintainers.

So for some reason, the script fail to parse the patches to find which
files are modified. And I think I know why. Usually, `git format-patch`
output something like:
diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index 4369fef161..9ffdd50c69 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
with all path starting with "a/" or "b/", which get_maintainer then
strip. (It strip them from the "diff --git" line, but remove the first
"directory" from the path for line starting with "---" or "+++".)

The patches in this series instead look like this:
diff --git tools/libs/light/libxl_dm.c tools/libs/light/libxl_dm.c
index 4369fef161..9ffdd50c69 100644
--- tools/libs/light/libxl_dm.c
+++ tools/libs/light/libxl_dm.c
with "a/" and "b/" missing, and that confuses get_maintainer.pl. It
tries to find maintainers for "libs/light/libxl_dm.c" which don't exist.

Did you by any chance used "--no-prefix"? It doesn't seems worse it to
handle this case as I'm guessing more than on tool rely on those.

Cheers,

--
Anthony PERARD

On Thu, 11 Apr 2024 at 14:24, Anthony PERARD <anthony.perard@cloud.com> wrote:
>
> On Thu, Apr 11, 2024 at 01:23:07PM +0300, Manos Pitsidianakis wrote:
> > Hello Anthony,
> >
> > I do know about that, in fact I used it and it did not output your
> > email. Here's what the `get_maintainer.pl` outputs: (same output for
> > all patches):
> >
> > $ ./scripts/get_maintainer.pl --no-l
> > ../../patches/libxl-dm-timeout-v1/v1-0001-libs-light-add-device-model-start-timeout-env-var.patch
> > Andrew Cooper <andrew.cooper3@citrix.com>
> > George Dunlap <george.dunlap@citrix.com>
> > Jan Beulich <jbeulich@suse.com>
> > Julien Grall <julien@xen.org>
> > Stefano Stabellini <sstabellini@kernel.org>
>
> Oh, sorry, I didn't realise this was "THE REST" that is the list of
> default maintainers.
>
> So for some reason, the script fail to parse the patches to find which
> files are modified. And I think I know why. Usually, `git format-patch`
> output something like:
> diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
> index 4369fef161..9ffdd50c69 100644
> --- a/tools/libs/light/libxl_dm.c
> +++ b/tools/libs/light/libxl_dm.c
> with all path starting with "a/" or "b/", which get_maintainer then
> strip. (It strip them from the "diff --git" line, but remove the first
> "directory" from the path for line starting with "---" or "+++".)
>
> The patches in this series instead look like this:
> diff --git tools/libs/light/libxl_dm.c tools/libs/light/libxl_dm.c
> index 4369fef161..9ffdd50c69 100644
> --- tools/libs/light/libxl_dm.c
> +++ tools/libs/light/libxl_dm.c
> with "a/" and "b/" missing, and that confuses get_maintainer.pl. It
> tries to find maintainers for "libs/light/libxl_dm.c" which don't exist.
>
> Did you by any chance used "--no-prefix"? It doesn't seems worse it to
> handle this case as I'm guessing more than on tool rely on those.
>
> Cheers,
>
> --
> Anthony PERARD


Mystery solved: I generated those patches on my arm64 workstation,
which had the debian packaged git version 2.39 which has the old
behavior of git-format-patch being affected by diff.noprefix. I just
re-generated them using 2.44 (which is what my config expected) and
the a/b prefixes are back:

./scripts/get_maintainer.pl
../../patches/libxl-dm-timeout-v2/v2-0002-libs-light-expand-device-model-start-timeout-use.patch
Anthony PERARD <anthony.perard@citrix.com>
Juergen Gross <jgross@suse.com>
xen-devel@lists.xenproject.org

I will send a v2, thanks for letting me know!


--
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd