| Message ID | 20190723140445.12748-3-sgarzare@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | pc: mmap kernel (ELF image) and initrd | expand |
On 23/07/19 16:04, Stefano Garzarella wrote: > In order to reduce the memory footprint we map into memory > the initrd using g_mapped_file_new() instead of reading it. > In this way we can share the initrd pages between multiple > instances of QEMU. > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Coverity is going to complain about the leaks. Should we instead store the initrd GMappedFile in PCMachineState, even if it is just for reference? Paolo > --- > hw/i386/pc.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 549c437050..b139589777 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1241,17 +1241,20 @@ static void load_linux(PCMachineState *pcms, > > /* load initrd */ > if (initrd_filename) { > + GMappedFile *gmf; > gsize initrd_size; > gchar *initrd_data; > GError *gerr = NULL; > > - if (!g_file_get_contents(initrd_filename, &initrd_data, > - &initrd_size, &gerr)) { > + gmf = g_mapped_file_new(initrd_filename, false, &gerr); > + if (!gmf) { > fprintf(stderr, "qemu: error reading initrd %s: %s\n", > initrd_filename, gerr->message); > exit(1); > } > > + initrd_data = g_mapped_file_get_contents(gmf); > + initrd_size = g_mapped_file_get_length(gmf); > initrd_max = pcms->below_4g_mem_size - pcmc->acpi_data_size - 1; > if (initrd_size >= initrd_max) { > fprintf(stderr, "qemu: initrd is too large, cannot support." > @@ -1378,6 +1381,7 @@ static void load_linux(PCMachineState *pcms, > > /* load initrd */ > if (initrd_filename) { > + GMappedFile *gmf; > gsize initrd_size; > gchar *initrd_data; > GError *gerr = NULL; > @@ -1387,12 +1391,15 @@ static void load_linux(PCMachineState *pcms, > exit(1); > } > > - if (!g_file_get_contents(initrd_filename, &initrd_data, > - &initrd_size, &gerr)) { > + gmf = g_mapped_file_new(initrd_filename, false, &gerr); > + if (!gmf) { > fprintf(stderr, "qemu: error reading initrd %s: %s\n", > initrd_filename, gerr->message); > exit(1); > } > + > + initrd_data = g_mapped_file_get_contents(gmf); > + initrd_size = g_mapped_file_get_length(gmf); > if (initrd_size >= initrd_max) { > fprintf(stderr, "qemu: initrd is too large, cannot support." > "(max: %"PRIu32", need %"PRId64")\n", >
On Tue, Jul 23, 2019 at 04:30:17PM +0200, Paolo Bonzini wrote: > On 23/07/19 16:04, Stefano Garzarella wrote: > > In order to reduce the memory footprint we map into memory > > the initrd using g_mapped_file_new() instead of reading it. > > In this way we can share the initrd pages between multiple > > instances of QEMU. > > > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > > Coverity is going to complain about the leaks. Should we instead store > the initrd GMappedFile in PCMachineState, even if it is just for reference? > Yes, I'll put it in the PCMachineState. Thanks, Stefano
diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 549c437050..b139589777 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1241,17 +1241,20 @@ static void load_linux(PCMachineState *pcms, /* load initrd */ if (initrd_filename) { + GMappedFile *gmf; gsize initrd_size; gchar *initrd_data; GError *gerr = NULL; - if (!g_file_get_contents(initrd_filename, &initrd_data, - &initrd_size, &gerr)) { + gmf = g_mapped_file_new(initrd_filename, false, &gerr); + if (!gmf) { fprintf(stderr, "qemu: error reading initrd %s: %s\n", initrd_filename, gerr->message); exit(1); } + initrd_data = g_mapped_file_get_contents(gmf); + initrd_size = g_mapped_file_get_length(gmf); initrd_max = pcms->below_4g_mem_size - pcmc->acpi_data_size - 1; if (initrd_size >= initrd_max) { fprintf(stderr, "qemu: initrd is too large, cannot support." @@ -1378,6 +1381,7 @@ static void load_linux(PCMachineState *pcms, /* load initrd */ if (initrd_filename) { + GMappedFile *gmf; gsize initrd_size; gchar *initrd_data; GError *gerr = NULL; @@ -1387,12 +1391,15 @@ static void load_linux(PCMachineState *pcms, exit(1); } - if (!g_file_get_contents(initrd_filename, &initrd_data, - &initrd_size, &gerr)) { + gmf = g_mapped_file_new(initrd_filename, false, &gerr); + if (!gmf) { fprintf(stderr, "qemu: error reading initrd %s: %s\n", initrd_filename, gerr->message); exit(1); } + + initrd_data = g_mapped_file_get_contents(gmf); + initrd_size = g_mapped_file_get_length(gmf); if (initrd_size >= initrd_max) { fprintf(stderr, "qemu: initrd is too large, cannot support." "(max: %"PRIu32", need %"PRId64")\n",
In order to reduce the memory footprint we map into memory the initrd using g_mapped_file_new() instead of reading it. In this way we can share the initrd pages between multiple instances of QEMU. Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- hw/i386/pc.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)