Re: [Qemu-devel] [PATCH] simple firmware.json test tool

[Philippe Mathieu-Daudé]

Hi Gerd,

On 20/4/18 17:38, Laszlo Ersek wrote:
> On 04/20/18 12:47, Gerd Hoffmann wrote:
> > applies on top of the firmware.json v2 series.
> > ---
> >   configure       |  2 +-
> >   Makefile        |  2 ++
> >   qemu-firmware.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 84 insertions(+), 1 deletion(-)
> >   create mode 100644 qemu-firmware.c
> >
> > diff --git a/configure b/configure
> > index 0a19b033bc..753f80147b 100755
> > --- a/configure
> > +++ b/configure
> > @@ -5495,7 +5495,7 @@ fi
> >   
> >   tools=""
> >   if test "$want_tools" = "yes" ; then
> > -  tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools"
> > +  tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) qemu-firmware\$(EXESUF) $tools"
> >     if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
> >       tools="qemu-nbd\$(EXESUF) $tools"
> >     fi
> > diff --git a/Makefile b/Makefile
> > index 32034abe15..4d6e627113 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -543,6 +543,8 @@ qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o 
> > $(COMMON_LDADDS)
> >   
> >   qemu-keymap$(EXESUF): qemu-keymap.o ui/input-keymap.o $(COMMON_LDADDS)
> >   
> > +qemu-firmware$(EXESUF): qemu-firmware.o $(COMMON_LDADDS)
> > +
> >   fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o 
> > fsdev/9p-marshal.o fsdev/9p-iov-marshal.o $(COMMON_LDADDS)
> >   fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
> >   
> > diff --git a/qemu-firmware.c b/qemu-firmware.c
> > new file mode 100644
> > index 0000000000..792f0fec8f
> > --- /dev/null
> > +++ b/qemu-firmware.c
> > @@ -0,0 +1,81 @@
> > +#include "qemu/osdep.h"
> > +#include "qemu-common.h"
> > +#include "qemu/error-report.h"
> > +
> > +#include "qapi/error.h"
> > +#include "qapi/qapi-types-firmware.h"
> > +#include "qapi/qapi-visit-firmware.h"
> > +#include "qapi/qobject-input-visitor.h"
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +    Error *err = NULL;
> > +    FirmwareMappingFlash *flash;
> > +    Firmware *fw;
> > +    Visitor *v;
> > +    int fd, size, rc;
> > +    char *buf;
> > +
> > +    if (argc != 2) {
> > +        fprintf(stderr, "usage: qemu-firmware <firmware-desc.json>\n");
> > +        exit(1);
> > +    }
> > +
> > +    /* read file */
> > +    fd = open(argv[1], O_RDONLY);
> > +    if (fd < 0) {
> > +        fprintf(stderr, "open %s: %s\n", argv[1], strerror(errno));
> > +        exit(1);
> > +    }
> > +    size = lseek(fd, 0, SEEK_END);
> > +    if (size < 0) {
> > +        perror("lseek");
> > +        exit(1);
> > +    }
> > +    lseek(fd, 0, SEEK_SET);
> > +    buf = malloc(size+1);
> > +    rc = read(fd, buf, size);
> > +    if (rc != size) {
> > +        fprintf(stderr, "file read error\n");
> > +        exit(1);
> > +    }
> > +    buf[size] = 0;
> > +    close(fd);
> > +
> > +    /* parse file */
> > +    v = qobject_input_visitor_new_str(buf, "", &err);
> > +    if (!v) {
> > +        error_report_err(err);
> > +        exit(1);
> > +    }
> > +    visit_type_Firmware(v, NULL, &fw, &error_fatal);
> > +    visit_free(v);
> > +
> > +    /* print cmdline */
> > +    switch (fw->mapping->device) {
> > +    case FIRMWARE_DEVICE_FLASH:
> > +        /*
> > +         * FIXME: nvram should be a per-guest copy.
> > +         *        How to handle that best here?
> > +         */
>
> Perhaps print a few shell commands first? Such as:
>
>    (
>      VARSTORE=$(mktemp)
>      trap 'rm -f -- "$VARSTORE"' EXIT
>      cat -- '[VARSTORE_TEMPLATE]' >> "$VARSTORE"
>      qemu ...
>    )
>
> It really does take separate actions, just like when you create a new
> disk image with "qemu-img".
>
> Thanks,
> Laszlo
>
> > +        flash = &fw->mapping->u.flash;
> > +        printf("-drive if=pflash,index=0,format=%s,file=%s\n",
> > +               BlockdevDriver_str(flash->executable->format),
> > +               flash->executable->pathname);
> > +        printf("-drive if=pflash,index=1,format=%s,file=%s\n",
> > +               BlockdevDriver_str(flash->nvram_template->format),
> > +               flash->nvram_template->pathname);
> > +        break;
> > +    case FIRMWARE_DEVICE_MEMORY:
> > +        printf("-bios %s\n", fw->mapping->u.memory.pathname);
> > +        break;
> > +    case FIRMWARE_DEVICE_KERNEL:
> > +        printf("-kernel %s\n", fw->mapping->u.kernel.pathname);
> > +        break;
> > +    default:
> > +        fprintf(stderr, "TODO\n");
> > +        break;
> > +    }
> > +
> > +    exit(0);
> > +}

Any plan to respin this patch?

Thanks,

Phil.