Re: [PATCH V6 13/14] tests/qtest: bootfile per vm
On 12/4/2023 5:37 PM, Peter Xu wrote: > On Mon, Dec 04, 2023 at 06:13:36PM -0300, Fabiano Rosas wrote: >> Steve Sistare writes: >> >>> Create a separate bootfile for the outgoing and incoming vm, so the block >>> layer can lock the file during the background migration test. Otherwise, >>> the test fails with: >>> "Failed to get "write" lock. Is another process using the image >>>[/tmp/migration-test-WAKPD2/bootsect]?" >> >> Hm.. what is the background migration even trying to access on the boot >> disk? @Peter? > > I didn't yet notice this patch until you asked, but background snapshot is > not designed to be used like this, afaict. > > It should normally be used when someone would like to use "savevm", then > background snapshot makes that snapshot save happen with VM running (live) > and mostly as performant as "savevm" due to page write protections (IOW, it > is not dirty tracking, but wr-protect each page so not writtable at all > until unprotected). > > Another difference (from "savevm") is, instead of storing that image onto > the block images, it stores that image also separately just like migrating > with "file:" as of now. > > When the dest QEMU starts it'll try to grab the image lock already because > it should never run with src running, just like when "loadvm" QEMU doesn't > assume the QEMU that ran "savevm" will be running. > >> >> This might be a good use for the -snapshot option. It should stop any >> attempt to get the write lock. Not a lot of difference, but slightly >> simpler. > > We don't yet have a background-snapshot test case. If we ever need, > that'll need to be done in two steps: start src, save snapshot into file, > start dest, load from snapshot file. We just shouldn't boot two together. > > Now after two years when I re-read the snapshot code a bit, I didn't even > find where QEMU took the disk snapshots.. logically it should be done at > the start of live background snapshot when VM was dumping device states, > something like bdrv_all_can_snapshot() orshould be needed to make sure all > images support snapshot on its own or it should already fail, and take > snapshots to match the image. > > IOW, I don't even think current raw disk would be able to support > background snapshot at all, otherwise if VM is live I don't see a way to > match the image (which is still lively updated by the running VM) to a live > snapshot taken. Copy the author, Andrey, for this question. > > Before that is confirmed, maybe the easiest way is we can go without a > background snapshot test case for suspend vm scenario. I am OK with dropping the test patches. It's a lot of change for a trivial test. tests/qtest: background migration with suspend tests/qtest: bootfile per vm - Steve
Re: [PATCH V6 13/14] tests/qtest: bootfile per vm
On Mon, Dec 04, 2023 at 06:13:36PM -0300, Fabiano Rosas wrote: > Steve Sistare writes: > > > Create a separate bootfile for the outgoing and incoming vm, so the block > > layer can lock the file during the background migration test. Otherwise, > > the test fails with: > > "Failed to get "write" lock. Is another process using the image > >[/tmp/migration-test-WAKPD2/bootsect]?" > > Hm.. what is the background migration even trying to access on the boot > disk? @Peter? I didn't yet notice this patch until you asked, but background snapshot is not designed to be used like this, afaict. It should normally be used when someone would like to use "savevm", then background snapshot makes that snapshot save happen with VM running (live) and mostly as performant as "savevm" due to page write protections (IOW, it is not dirty tracking, but wr-protect each page so not writtable at all until unprotected). Another difference (from "savevm") is, instead of storing that image onto the block images, it stores that image also separately just like migrating with "file:" as of now. When the dest QEMU starts it'll try to grab the image lock already because it should never run with src running, just like when "loadvm" QEMU doesn't assume the QEMU that ran "savevm" will be running. > > This might be a good use for the -snapshot option. It should stop any > attempt to get the write lock. Not a lot of difference, but slightly > simpler. We don't yet have a background-snapshot test case. If we ever need, that'll need to be done in two steps: start src, save snapshot into file, start dest, load from snapshot file. We just shouldn't boot two together. Now after two years when I re-read the snapshot code a bit, I didn't even find where QEMU took the disk snapshots.. logically it should be done at the start of live background snapshot when VM was dumping device states, something like bdrv_all_can_snapshot() orshould be needed to make sure all images support snapshot on its own or it should already fail, and take snapshots to match the image. IOW, I don't even think current raw disk would be able to support background snapshot at all, otherwise if VM is live I don't see a way to match the image (which is still lively updated by the running VM) to a live snapshot taken. Copy the author, Andrey, for this question. Before that is confirmed, maybe the easiest way is we can go without a background snapshot test case for suspend vm scenario. Thanks, -- Peter Xu
Re: [PATCH V6 13/14] tests/qtest: bootfile per vm
Steve Sistare writes: > Create a separate bootfile for the outgoing and incoming vm, so the block > layer can lock the file during the background migration test. Otherwise, > the test fails with: > "Failed to get "write" lock. Is another process using the image >[/tmp/migration-test-WAKPD2/bootsect]?" Hm.. what is the background migration even trying to access on the boot disk? @Peter? This might be a good use for the -snapshot option. It should stop any attempt to get the write lock. Not a lot of difference, but slightly simpler.
