[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 27/34] migration/multifd: Move nocomp code into multifd-nocomp
From: |
Peter Maydell |
Subject: |
Re: [PULL 27/34] migration/multifd: Move nocomp code into multifd-nocomp.c |
Date: |
Mon, 9 Sep 2024 11:37:10 +0100 |
On Mon, 9 Sept 2024 at 11:28, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Wed, 4 Sept 2024 at 13:48, Fabiano Rosas <farosas@suse.de> wrote:
> >
> > In preparation for adding new payload types to multifd, move most of
> > the no-compression code into multifd-nocomp.c. Let's try to keep a
> > semblance of layering by not mixing general multifd control flow with
> > the details of transmitting pages of ram.
> >
> > There are still some pieces leftover, namely the p->normal, p->zero,
> > etc variables that we use for zero page tracking and the packet
> > allocation which is heavily dependent on the ram code.
> >
> > Reviewed-by: Peter Xu <peterx@redhat.com>
> > Signed-off-by: Fabiano Rosas <farosas@suse.de>
>
> I know Coverity has only flagged this up because the
> code has moved, but it seems like a good place to ask
> the question:
>
> > +void multifd_ram_fill_packet(MultiFDSendParams *p)
> > +{
> > + MultiFDPacket_t *packet = p->packet;
> > + MultiFDPages_t *pages = &p->data->u.ram;
> > + uint32_t zero_num = pages->num - pages->normal_num;
> > +
> > + packet->pages_alloc = cpu_to_be32(multifd_ram_page_count());
> > + packet->normal_pages = cpu_to_be32(pages->normal_num);
> > + packet->zero_pages = cpu_to_be32(zero_num);
> > +
> > + if (pages->block) {
> > + strncpy(packet->ramblock, pages->block->idstr, 256);
>
> Coverity points out that when we fill in the RAMBlock::idstr
> here, if packet->ramblock is not NUL terminated then we won't
> NUL-terminate idstr either (CID 1560071).
>
> Is this really what is intended?
>
> Perhaps
> pstrncpy(packet->ramblock, sizeof(packet->ramblock),
> pages->block->idstr);
>
> would be better?
>
> (pstrncpy will always NUL-terminate, and won't pointlessly
> zero-fill the space after the string in the destination.)
Whoops, the name of the function I'm trying to recommend
is "pstrcpy", not "pstrncpy" :-)
-- PMM
- [PULL 19/34] migration/multifd: Move pages accounting into multifd_send_zero_page_detect(), (continued)
- [PULL 19/34] migration/multifd: Move pages accounting into multifd_send_zero_page_detect(), Fabiano Rosas, 2024/09/04
- [PULL 20/34] migration/multifd: Remove total pages tracing, Fabiano Rosas, 2024/09/04
- [PULL 21/34] migration/multifd: Isolate ram pages packet data, Fabiano Rosas, 2024/09/04
- [PULL 22/34] migration/multifd: Don't send ram data during SYNC, Fabiano Rosas, 2024/09/04
- [PULL 23/34] migration/multifd: Replace multifd_send_state->pages with client data, Fabiano Rosas, 2024/09/04
- [PULL 24/34] migration/multifd: Allow multifd sync without flush, Fabiano Rosas, 2024/09/04
- [PULL 25/34] migration/multifd: Standardize on multifd ops names, Fabiano Rosas, 2024/09/04
- [PULL 26/34] migration/multifd: Register nocomp ops dynamically, Fabiano Rosas, 2024/09/04
- [PULL 27/34] migration/multifd: Move nocomp code into multifd-nocomp.c, Fabiano Rosas, 2024/09/04
- [PULL 29/34] migration/multifd: Stop changing the packet on recv side, Fabiano Rosas, 2024/09/04
- [PULL 28/34] migration/multifd: Make MultiFDMethods const, Fabiano Rosas, 2024/09/04
- [PULL 30/34] migration/multifd: Fix p->iov leak in multifd-uadk.c, Fabiano Rosas, 2024/09/04
- [PULL 33/34] target/ppc: Fix migration of CPUs with TLB_EMB TLB type, Fabiano Rosas, 2024/09/04
- [PULL 31/34] migration/multifd: Add a couple of asserts for p->iov, Fabiano Rosas, 2024/09/04
- [PULL 32/34] migration/multifd: Add documentation for multifd methods, Fabiano Rosas, 2024/09/04
- [PULL 34/34] tests/qtest/migration: Add a check for the availability of the "pc" machine, Fabiano Rosas, 2024/09/04
- Re: [PULL 00/34] Migration patches for 2024-09-04, Peter Maydell, 2024/09/05