search

void-linux / void-packages

The Void source packages collection
https://voidlinux.org
Other
2.59k stars 2.16k forks source link

qemu: datadir is not set, uefi support is broken #27965

Closed gspe closed 5 months ago

gspe commented 3 years ago

this pr https://github.com/void-linux/void-packages/pull/24884 remove --datadir=/usr/lib but not set --datadir=/usr/share so now qemu package has broken support for uefi, for example this is the content of 60-edk2-x86_64.json :

cat /usr/share/qemu/firmware/60-edk2-x86_64.json
{
    "description": "UEFI firmware for x86_64",
    "interface-types": [
        "uefi"
    ],
    "mapping": {
        "device": "flash",
        "executable": {
            "filename": "share/qemu/edk2-x86_64-code.fd",
            "format": "raw"
        },
        "nvram-template": {
            "filename": "share/qemu/edk2-i386-vars.fd",
            "format": "raw"
        }
    },
    "targets": [
        {
            "architecture": "x86_64",
            "machines": [
                "pc-i440fx-*",
                "pc-q35-*"
            ]
        }
    ],
    "features": [
        "acpi-s3",
        "amd-sev",
        "verbose-dynamic"
    ],
    "tags": [

    ]
}

as you can see the filename path is not correct "filename": "share/qemu/edk2-i386-vars.fd",

ericonr commented 3 years ago

@gspe do you think you can make a PR to fix this?

gspe commented 3 years ago

I'll make a PR tomorrow.

ericonr commented 3 years ago

For the record, qemu has switched to meson but hid that by including a copy of meson in their source tree and calling that from their 2k line configure script (their meson.build also has 2k lines, yay).

If you want a challenge, you can try to move qemu over to the meson build style.

gspe commented 3 years ago

@ericonr I've tried to make a PR for qemu but I have a problem: building the package fails tests. I have updated my void-packages repo and tried to build the original qemu in void-packages without any changes and it also fails test.

Regarding the meson build style I have read some documentation and according with the changelog https://wiki.qemu.org/ChangeLog/5.2#Build_Information the switch to meson is not complete at the moment:

The build system is now partly based on Meson. However, building is still done with configure and make as in previous versions of QEMU.

indeed some building options are still not present in meson_options.txt file.

ericonr commented 3 years ago

building the package fails tests.

I doubt some can even be run on a container, so it should be fine to gate them with "$XBPS_CHECK_PKGS" = full

And ok, we can stick with configure for now.

gspe commented 3 years ago

I've used "$XBPS_CHECK_PKGS" = full option but it doesn't make any difference I still got errors during checks this is pastebin of them https://pastebin.com/VUCnbbTY

This is just one of them in case of pastebin doesn't works

TEST    iotest-qcow2: 229 [fail]
QEMU          -- "/builddir/qemu-5.2.0/build/tests/qemu-iotests/../../qemu-system-x86_64" -nodefaults -display none -accel qtest
QEMU_IMG      -- "/builddir/qemu-5.2.0/build/tests/qemu-iotests/../../qemu-img"
QEMU_IO       -- "/builddir/qemu-5.2.0/build/tests/qemu-iotests/../../qemu-io"  --cache writeback --aio threads -f qcow2
QEMU_NBD      -- "/builddir/qemu-5.2.0/build/tests/qemu-iotests/../../qemu-nbd"
IMGFMT        -- qcow2 (compat=1.1)
IMGPROTO      -- file
PLATFORM      -- Linux/x86_64  5.9.16_1
TEST_DIR      -- /builddir/qemu-5.2.0/build/tests/qemu-iotests/scratch
SOCK_DIR      -- /tmp/tmp.gRja3PqgOn
SOCKET_SCM_HELPER -- /builddir/qemu-5.2.0/build/tests/qemu-iotests/socket_scm_helper

--- /builddir/qemu-5.2.0/tests/qemu-iotests/229.out 2020-12-08 17:59:44.000000000 +0100
+++ /builddir/qemu-5.2.0/build/tests/qemu-iotests/229.out.bad   2021-01-21 10:50:21.549119104 +0100
@@ -8,7 +8,14 @@

 === Starting drive-mirror, causing error & stop  ===

-{'execute': 'drive-mirror', 'arguments': {'device': 'testdisk', 'format': 'IMGFMT', 'target': 'blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/d.IMGFMT', 'sync': 'full', 'mode': 'existing', 'on-source-error': 'stop', 'on-target-error': 'stop' }}
+{'execute': 'drive-mirror',
+                 'arguments': {'device': 'testdisk',
+                               'format': 'IMGFMT',
+                               'target': 'blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/d.IMGFMT',
+                               'sync':   'full',
+                               'mode':   'existing',
+                               'on-source-error': 'stop',
+                               'on-target-error': 'stop' }}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "testdisk"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "testdisk"}}
 {"return": {}}
@@ -17,7 +24,9 @@

 === Force cancel job paused in error state  ===

-{'execute': 'block-job-cancel', 'arguments': { 'device': 'testdisk', 'force': true}}
+{'execute': 'block-job-cancel',
+                 'arguments': { 'device': 'testdisk',
+                                'force': true}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "testdisk"}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "aborting", "id": "testdisk"}}
  TEST    iotest-qcow2: 244
  TEST    iotest-qcow2: 249 [fail]
QEMU          -- "/builddir/qemu-5.2.0/build/tests/qemu-iotests/../../qemu-system-x86_64" -nodefaults -display none -accel qtest
QEMU_IMG      -- "/builddir/qemu-5.2.0/build/tests/qemu-iotests/../../qemu-img"
QEMU_IO       -- "/builddir/qemu-5.2.0/build/tests/qemu-iotests/../../qemu-io"  --cache writeback --aio threads -f qcow2
QEMU_NBD      -- "/builddir/qemu-5.2.0/build/tests/qemu-iotests/../../qemu-nbd"
IMGFMT        -- qcow2 (compat=1.1)
IMGPROTO      -- file
PLATFORM      -- Linux/x86_64  5.9.16_1
TEST_DIR      -- /builddir/qemu-5.2.0/build/tests/qemu-iotests/scratch
SOCK_DIR      -- /tmp/tmp.gRja3PqgOn
SOCKET_SCM_HELPER -- /builddir/qemu-5.2.0/build/tests/qemu-iotests/socket_scm_helper

--- /builddir/qemu-5.2.0/tests/qemu-iotests/249.out 2020-12-08 17:59:44.000000000 +0100
+++ /builddir/qemu-5.2.0/build/tests/qemu-iotests/249.out.bad   2021-01-21 10:50:22.114119114 +0100
@@ -7,24 +7,29 @@

 === Send a write command to a drive opened in read-only mode (1)

-{ 'execute': 'human-monitor-command', 'arguments': {'command-line': 'qemu-io none0 "aio_write 0 2k"'}}
+{ 'execute': 'human-monitor-command',
+       'arguments': {'command-line': 'qemu-io none0 "aio_write 0 2k"'}}
 {"return": "Block node is read-onlyrn"}

 === Run block-commit on base using an invalid filter node name

-{ 'execute': 'block-commit', 'arguments': {'job-id': 'job0', 'device': 'none1', 'top-node': 'int', 'filter-node-name': '1234'}}
+{ 'execute': 'block-commit',
+       'arguments': {'job-id': 'job0', 'device': 'none1', 'top-node': 'int',
+                     'filter-node-name': '1234'}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "job0"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "job0"}}
 {"error": {"class": "GenericError", "desc": "Invalid node name"}}

 === Send a write command to a drive opened in read-only mode (2)

-{ 'execute': 'human-monitor-command', 'arguments': {'command-line': 'qemu-io none0 "aio_write 0 2k"'}}
+{ 'execute': 'human-monitor-command',
+       'arguments': {'command-line': 'qemu-io none0 "aio_write 0 2k"'}}
 {"return": "Block node is read-onlyrn"}

 === Run block-commit on base using the default filter node name

-{ 'execute': 'block-commit', 'arguments': {'job-id': 'job0', 'device': 'none1', 'top-node': 'int'}}
+{ 'execute': 'block-commit',
+       'arguments': {'job-id': 'job0', 'device': 'none1', 'top-node': 'int'}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "job0"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "job0"}}
 {"return": {}}
@@ -36,6 +41,7 @@

 === Send a write command to a drive opened in read-only mode (3)

-{ 'execute': 'human-monitor-command', 'arguments': {'command-line': 'qemu-io none0 "aio_write 0 2k"'}}
+{ 'execute': 'human-monitor-command',
+       'arguments': {'command-line': 'qemu-io none0 "aio_write 0 2k"'}}
 {"return": "Block node is read-onlyrn"}
 *** done
  TEST    iotest-qcow2: 251
  TEST    iotest-qcow2: 252
  TEST    iotest-qcow2: 256
  TEST    iotest-qcow2: 259 [not run]
  TEST    iotest-qcow2: 265
  TEST    iotest-qcow2: 267
  TEST    iotest-qcow2: 268
  TEST    iotest-qcow2: 271
  TEST    iotest-qcow2: 283
  TEST    iotest-qcow2: 287
  TEST    iotest-qcow2: 290
  TEST    iotest-qcow2: 292
  TEST    iotest-qcow2: 299
  TEST    iotest-qcow2: 309
Not run: 259
Failures: 117 127 140 141 143 156 161 191 229 249
Failed 10 of 122 iotests
make[1]: *** [/builddir/qemu-5.2.0/tests/Makefile.include:144: check-block] Error 1
make[1]: Leaving directory '/builddir/qemu-5.2.0/build'
make: *** [GNUmakefile:11: check] Error 2
=> ERROR: qemu-5.2.0_1: do_check: '${make_cmd} ${make_check_args} ${make_check_target}' exited with 2
=> ERROR:   in do_check() at common/build-style/configure.sh:32

Maybe depends on my hardware, I'm using a ryzen 7 or maybe it's better to wait that the c/c++ upgrade is finished.

ericonr commented 3 years ago

How are you running the build?

Also what's the template looking like?

gspe commented 3 years ago

I'm running the standard command to build packages ./xbps-src -f pkg qemu

The template is the original one in srcpkgs, I haven't made any changes to it for this test.

ericonr commented 3 years ago

Do you have anything about XBPS_CHECK_PKGS in etc/conf?

gspe commented 3 years ago

This is my etc/conf

cat etc/conf
# [OPTIONAL]
# Enable building package locally that are restricted legally for redistribution.
# NOTE: you can't distribute the sources or binaries for such kind of packages.
#
XBPS_ALLOW_RESTRICTED=yes

# [OPTIONAL]
# Number of parallel jobs to execute when building packages that
# use make(1) or alike commands.
#
XBPS_MAKEJOBS=16

# [OPTIONAL]
# Enable running the (optional) do_check() function of a package.
# When set to 'full', will enable further testing for some packages.
#
# XBPS_CHECK_PKGS=yes
XBPS_CHECK_PKGS=full
ericonr commented 3 years ago

XBPS_CHECK_PKGS=full

Well there you go :P

It's always running full tests. You can run them selectively with -K, that way you don't need that flag in etc/conf.

ackalker commented 3 years ago

Back on topic for this issue: I've had a look at the Qemu source, in particular the meson.build file I suspect is responsible for the problem: pc-bios/descriptors/meson.build#L11

I don't see any particular problem with it, but I'm in no way a Meson expert.

AFAICT the configure_file action is supposed to take a file, change any references to @DATADIR@ in its contents to the value of qemu_datadir, and install it into the directory named by the contatenation of the value of qemu_datadir, a /, and the string firmware, which seems mostly what is happening during the build. Files get installed into /usr/share/qemu/firmware (ignoring $DESTDIR for now), except for some reason @DATADIR@ gets substituted with share/qemu and not /usr/share/qemu, i.e. it is missing the prefix: /usr. I find this very strange, as in other places, qemu_datadir does seem to evaluate to /usr/share/qemu. Is there maybe some bad interaction going on here? Is there perhaps an environment variable DATADIR whose value is overriding that of the substitution?

ackalker commented 3 years ago

I believe that the following 2 commits to be relevant to the current issue: qemu/qemu@6aae2a2e0e3c43e479f889389074cda8bef3a580, qemu/qemu@ab4c0996f80d43d1fc28c6e76f4ecb27423a7e29.

In particular, the commit message of the 2nd commit seems relevant:

When cross-compiling, by default qemu_datadir is 'c:\Program Files\QEMU', which is not recognized as being an absolute path, and meson will end up adding the prefix again.

From the documentation for configure_file(), the problem becomes obvious:

Trying to use variations using the same variable qemu_datadir for both, we end up with a Catch-22: when its value is an absolute Windows path, then Meson doesn't recognize it as absolute when cross-compiling, and it adds a redundant prefix. When its value is a relative path, the resulting firmware descriptor files will contain relative paths and will not work.

ackalker commented 3 years ago

I've tested passing --datadir=<absolute path> to configure in a Docker test build, but that doesn't appear to work: the filename attributes in the descriptor files do get the full path, but they are missing the initial /, so the paths are still relative. I believe this needs to be fixed upstream, so I have created a bug report about this issue (with full testing steps for anyone interested): https://bugs.launchpad.net/qemu/+bug/1913012

classabbyamp commented 5 months ago

appears to work now:

cat /usr/share/qemu/firmware/60-edk2-x86_64.json
{
    "description": "UEFI firmware for x86_64",
    "interface-types": [
        "uefi"
    ],
    "mapping": {
        "device": "flash",
        "executable": {
            "filename": "/usr/share/qemu/edk2-x86_64-code.fd",
            "format": "raw"
        },
        "nvram-template": {
            "filename": "/usr/share/qemu/edk2-i386-vars.fd",
            "format": "raw"
        }
    },
    "targets": [
        {
            "architecture": "x86_64",
            "machines": [
                "pc-i440fx-*",
                "pc-q35-*"
            ]
        }
    ],
    "features": [
        "acpi-s3",
        "amd-sev",
        "verbose-dynamic"
    ],
    "tags": [

    ]
}