[External] Re: [PATCH v2 1/4] API: introduce memory failure

[Peter Krempa]

On Wed, Oct 14, 2020 at 10:44:53 +0800, zhenwei pi wrote:
> 
> 
> On 10/13/20 8:31 PM, Daniel Henrique Barboza wrote:
> > This patch failed to compile in my env:
> > 
> > 
> > FAILED: tools/virsh.p/virsh-domain.c.o
> > [....]
> > -D_FUNCTION_DEF -MD -MQ tools/virsh.p/virsh-domain.c.o -MF
> > tools/virsh.p/virsh-domain.c.o.d -o tools/virsh.p/virsh-domain.c.o -c
> > ../tools/virsh-domain.c
> > In file included from /usr/lib64/glib-2.0/include/glibconfig.h:9,
> >                   from /usr/include/glib-2.0/glib/gtypes.h:32,
> >                   from /usr/include/glib-2.0/glib/galloca.h:32,
> >                   from /usr/include/glib-2.0/glib.h:30,
> >                   from ../src/util/glibcompat.h:21,
> >                   from ../src/internal.h:30,
> >                   from ../tools/virsh.h:25,
> >                   from ../tools/virsh-domain.h:23,
> >                   from ../tools/virsh-domain.c:22:
> > /usr/include/glib-2.0/glib/gmacros.h:745:53: error: size of array
> > ‘_GStaticAssertCompileTimeAssertion_185’ is negative
> >    745 | #define G_STATIC_ASSERT(expr) typedef char G_PASTE
> > (_GStaticAssertCompileTimeAssertion_, __COUNTER__)[(expr) ? 1 : -1]
> > G_GNUC_UNUSED
> >        |
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > /usr/include/glib-2.0/glib/gmacros.h:735:47: note: in definition of
> > macro ‘G_PASTE_ARGS’
> >    735 | #define G_PASTE_ARGS(identifier1,identifier2) identifier1 ##
> > identifier2
> >        |                                               ^~~~~~~~~~~
> > /usr/include/glib-2.0/glib/gmacros.h:745:44: note: in expansion of macro
> > ‘G_PASTE’
> >    745 | #define G_STATIC_ASSERT(expr) typedef char G_PASTE
> > (_GStaticAssertCompileTimeAssertion_, __COUNTER__)[(expr) ? 1 : -1]
> > G_GNUC_UNUSED
> >        |                                            ^~~~~~~
> > ../tools/virsh-domain.c:13643:1: note: in expansion of macro
> > ‘G_STATIC_ASSERT’
> > 13643 | G_STATIC_ASSERT(VIR_DOMAIN_EVENT_ID_LAST ==
> > G_N_ELEMENTS(virshDomainEventCallbacks));
> >        | ^~~~~~~~~~~~~~~
> > [505/984] Compiling C object
> > src/virtqemud.p/remote_remote_daemon_dispatch.c.o
> > ninja: build stopped: subcommand failed.
> > $
> > 
> > 
> > I didn't verify if the following patches fixes it.
> > 
> > 
> > Thanks,
> > 
> > 
> > DHB
> > 
> 
> I described it in '[PATCH v2 4/4] virsh: implement memory failure event'
> 
> Notice:
> The full patch set includes 4 patches:
> virsh: implement memory failure event         (current patch)
> qemu: monitor: handle memory failure event
> qemu: process: implement domainMemoryFailure
> API: introduce memory failure
> 
> To avoid build/test errors, the 4 patches should be merged/removed
> together.

No, they just need to be moved into appropriate order so that they can
be built. There's no point splitting them just to merge them later.

> Suggested by Peter, separate a 'all in one' patch into 4 patches (described
> in cover letter '[PATCH v2 0/4] support memory failure').
> I forked a repo and pushed the 4
> patches(https://gitlab.com/pacepi/libvirt/-/tree/memory-failure-v2), CI
> worked fine.

Our contribution guidelines required that the tree builds successfully
after every single patch:

https://libvirt.org/hacking.html

Section "Preparing patches":

"If you're going to submit multiple patches, the automated tests must
pass *after each patch*, not just after the last one."