Message ID | 20200703201911.26573-1-f4bug@amsat.org |
---|---|
Headers | show |
Series | hw: Mark the device with no migratable fields | expand |
Patchew URL: https://patchew.org/QEMU/20200703201911.26573-1-f4bug@amsat.org/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === LINK tests/test-io-task LINK tests/test-io-channel-socket hw/core/qdev.o:(.data.rel+0x0): undefined reference to `vmstate_no_state_to_migrate' collect2: error: ld returned 1 exit status make: *** [tests/test-qdev-global-props] Error 1 make: *** Waiting for unfinished jobs.... TEST iotest-qcow2: 002 TEST iotest-qcow2: 003 --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=ba983f0b8bc94d39a0a649a449a91f08', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-mg_r93vd/src/docker-src.2020-07-03-16.46.29.22759:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=ba983f0b8bc94d39a0a649a449a91f08 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-mg_r93vd/src' make: *** [docker-run-test-quick@centos7] Error 2 real 16m8.994s user 0m8.902s The full log is available at http://patchew.org/logs/20200703201911.26573-1-f4bug@amsat.org/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
On Fri, 3 Jul 2020 at 21:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > This is a proof-of-concept after chatting with Peter Maydell > on IRC earlier. > > Introduce the vmstate_no_state_to_migrate structure, and > a reference to it: vmstate_qdev_no_state_to_migrate. > Use this reference in devices with no fields to migrate. > > This is useful to catch devices missing vmstate, such: > - ads7846 > - mcf-uart > - mcf-fec > - versatile_i2c > - ... > > I am not sure about: > - gpex-pcihost I think it's correct that this has no internal state: the only interesting state is in the GPEXRootState, which is a TYPE_GPEX_ROOT_DEVICE which migrates itself. I made some comments on the "meaty" bits of the patchset, and reviewed one or two of the "mark this device as having no migration state" patches, but it doesn't seem worth reviewing all of them until the migration submaintainers have a chance to weigh in on whether they like the concept (I expect they're busy right now with freeze-related stuff :-)) thanks -- PMM
On 7/9/20 9:19 PM, Peter Maydell wrote: > On Fri, 3 Jul 2020 at 21:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> >> This is a proof-of-concept after chatting with Peter Maydell >> on IRC earlier. >> >> Introduce the vmstate_no_state_to_migrate structure, and >> a reference to it: vmstate_qdev_no_state_to_migrate. >> Use this reference in devices with no fields to migrate. >> >> This is useful to catch devices missing vmstate, such: >> - ads7846 >> - mcf-uart >> - mcf-fec >> - versatile_i2c >> - ... >> >> I am not sure about: >> - gpex-pcihost > > I think it's correct that this has no internal state: > the only interesting state is in the GPEXRootState, which > is a TYPE_GPEX_ROOT_DEVICE which migrates itself. > > I made some comments on the "meaty" bits of the patchset, > and reviewed one or two of the "mark this device as > having no migration state" patches, but it doesn't seem > worth reviewing all of them until the migration submaintainers > have a chance to weigh in on whether they like the concept > (I expect they're busy right now with freeze-related stuff :-)) Now that we are far from freeze-date is a good time to ping again on this concept :) Most of the devices are ARM except: - cpu-cluster (Eduardo/Marcel) - hcd-ohci (Gerd) - mac-nubus-bridge (Laurent) - generic QOM (Daniel, Paolo) Is someone against this proposal? Regards, Phil.
Le 14/01/2021 à 16:49, Philippe Mathieu-Daudé a écrit : > On 7/9/20 9:19 PM, Peter Maydell wrote: >> On Fri, 3 Jul 2020 at 21:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >>> >>> This is a proof-of-concept after chatting with Peter Maydell >>> on IRC earlier. >>> >>> Introduce the vmstate_no_state_to_migrate structure, and >>> a reference to it: vmstate_qdev_no_state_to_migrate. >>> Use this reference in devices with no fields to migrate. >>> >>> This is useful to catch devices missing vmstate, such: >>> - ads7846 >>> - mcf-uart >>> - mcf-fec >>> - versatile_i2c >>> - ... >>> >>> I am not sure about: >>> - gpex-pcihost >> >> I think it's correct that this has no internal state: >> the only interesting state is in the GPEXRootState, which >> is a TYPE_GPEX_ROOT_DEVICE which migrates itself. >> >> I made some comments on the "meaty" bits of the patchset, >> and reviewed one or two of the "mark this device as >> having no migration state" patches, but it doesn't seem >> worth reviewing all of them until the migration submaintainers >> have a chance to weigh in on whether they like the concept >> (I expect they're busy right now with freeze-related stuff :-)) > > Now that we are far from freeze-date is a good time to ping > again on this concept :) > > Most of the devices are ARM except: > - cpu-cluster (Eduardo/Marcel) > - hcd-ohci (Gerd) > - mac-nubus-bridge (Laurent) > - generic QOM (Daniel, Paolo) > > Is someone against this proposal? I'm not against the proposal, but I don't understand why we need this. Thanks, Laurent
Hi Laurent, On 1/18/21 8:33 AM, Laurent Vivier wrote: > Le 14/01/2021 à 16:49, Philippe Mathieu-Daudé a écrit : >> On 7/9/20 9:19 PM, Peter Maydell wrote: >>> On Fri, 3 Jul 2020 at 21:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >>>> >>>> This is a proof-of-concept after chatting with Peter Maydell >>>> on IRC earlier. >>>> >>>> Introduce the vmstate_no_state_to_migrate structure, and >>>> a reference to it: vmstate_qdev_no_state_to_migrate. >>>> Use this reference in devices with no fields to migrate. >>>> >>>> This is useful to catch devices missing vmstate, such: >>>> - ads7846 >>>> - mcf-uart >>>> - mcf-fec >>>> - versatile_i2c >>>> - ... >>>> >>>> I am not sure about: >>>> - gpex-pcihost >>> >>> I think it's correct that this has no internal state: >>> the only interesting state is in the GPEXRootState, which >>> is a TYPE_GPEX_ROOT_DEVICE which migrates itself. >>> >>> I made some comments on the "meaty" bits of the patchset, >>> and reviewed one or two of the "mark this device as >>> having no migration state" patches, but it doesn't seem >>> worth reviewing all of them until the migration submaintainers >>> have a chance to weigh in on whether they like the concept >>> (I expect they're busy right now with freeze-related stuff :-)) >> >> Now that we are far from freeze-date is a good time to ping >> again on this concept :) >> >> Most of the devices are ARM except: >> - cpu-cluster (Eduardo/Marcel) >> - hcd-ohci (Gerd) >> - mac-nubus-bridge (Laurent) >> - generic QOM (Daniel, Paolo) >> >> Is someone against this proposal? > > I'm not against the proposal, but I don't understand why we need this. IIRC the IRC discussion followed this thread: https://www.mail-archive.com/qemu-devel@nongnu.org/msg554453.html Quoting Peter: > I think we should care about migration on all architectures > and devices, in the sense that we want savevm/loadvm to work. > This is a really useful debugging and user tool, and when > I'm reviewing devices it's the minimum bar I think new > devices should clear. You then get migration "for free" but > I don't particularly expect it to be used compared to > snapshot save/restore. (Of course some of our existing code > doesn't support this, and we don't have a good way of testing > so bugs creep in easily, but as a principle I think it's > good.) Currently there is no automatic way to catch missing vmstate, we rely on code review (mostly from Peter...). To be able to add a code check to catch the future device added, we need to first clean the (old) devices missing VMState, justifying why each doesn't have any field to migrate. Also IMO it is simpler to have an unified API, rather than explaining each experienced and new contributor why "old style qdev" are allowed to do things than "new introduced qdev" can't do anymore. Regards, Phil.