Re: [Qemu-block] [PATCH COLO v3 11/14] Backup: clear all bitmap when doing block checkpoint
On 04/03/2015 07:09 PM, Paolo Bonzini wrote:
>
>
> On 03/04/2015 12:01, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang
>> Signed-off-by: zhanghailiang
>> Signed-off-by: Gonglei
>> Cc: Jeff Cody
>> ---
>> block/backup.c | 13 +
>> blockjob.c | 10 ++
>> include/block/blockjob.h | 12
>> 3 files changed, 35 insertions(+)
>>
>> diff --git a/block/backup.c b/block/backup.c
>> index 1c535b1..e8b8931 100644
>> --- a/block/backup.c
>> +++ b/block/backup.c
>> @@ -208,11 +208,24 @@ static void backup_iostatus_reset(BlockJob *job)
>> bdrv_iostatus_reset(s->target);
>> }
>>
>> +static void backup_do_checkpoint(BlockJob *job, Error **errp)
>> +{
>> +BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common);
>> +
>> +if (backup_job->sync_mode != MIRROR_SYNC_MODE_NONE) {
>> +error_setg(errp, "this feature or command is not currently
>> supported");
>> +return;
>> +}
>> +
>> +hbitmap_reset_all(backup_job->bitmap);
>> +}
>> +
>> static const BlockJobDriver backup_job_driver = {
>> .instance_size = sizeof(BackupBlockJob),
>> .job_type = BLOCK_JOB_TYPE_BACKUP,
>> .set_speed = backup_set_speed,
>> .iostatus_reset = backup_iostatus_reset,
>> +.do_checkpoint = backup_do_checkpoint,
>> };
>>
>> static BlockErrorAction backup_error_action(BackupBlockJob *job,
>> diff --git a/blockjob.c b/blockjob.c
>> index ba2255d..dbac81e 100644
>> --- a/blockjob.c
>> +++ b/blockjob.c
>> @@ -388,3 +388,13 @@ void block_job_defer_to_main_loop(BlockJob *job,
>>
>> qemu_bh_schedule(data->bh);
>> }
>> +
>> +void block_job_do_checkpoint(BlockJob *job, Error **errp)
>> +{
>> +if (!job->driver->do_checkpoint) {
>> +error_setg(errp, "this feature or command is not currently
>> supported");
>> +return;
>> +}
>> +
>> +job->driver->do_checkpoint(job, errp);
>> +}
>> diff --git a/include/block/blockjob.h b/include/block/blockjob.h
>> index b6d4ebb..c6f1cad 100644
>> --- a/include/block/blockjob.h
>> +++ b/include/block/blockjob.h
>> @@ -50,6 +50,9 @@ typedef struct BlockJobDriver {
>> * manually.
>> */
>> void (*complete)(BlockJob *job, Error **errp);
>> +
>> +/** Optional callback for job types that support checkpoint. */
>> +void (*do_checkpoint)(BlockJob *job, Error **errp);
>> } BlockJobDriver;
>>
>> /**
>> @@ -334,4 +337,13 @@ void block_job_defer_to_main_loop(BlockJob *job,
>>BlockJobDeferToMainLoopFn *fn,
>>void *opaque);
>>
>> +/**
>> + * block_job_do_checkpoint:
>> + * @job: The job.
>> + * @errp: Error object.
>> + *
>> + * Do block checkpoint on the specified job.
>> + */
>> +void block_job_do_checkpoint(BlockJob *job, Error **errp);
>> +
>> #endif
>>
>
> Does this only run on the secondary, or also on the primary? What
> happens if you use a block job on the primary?
It is only for secondary. This new blockjob API is only called in qcow2+colo,
which is used for secondary qemu. So primary qemu will not come here.
If it happens, it is a bug. Should we check it here?
Thanks
Wen Congyang
>
> Perhaps a variant of backup_job_driver is needed for COLO, and the
> default behavior of block_job_do_checkpoint should be to do nothing?
>
> Paolo
> .
>
Re: [Qemu-block] [Qemu-devel] qemu-img behavior for locating backing files
On 04/02/2015 05:38 AM, Kevin Wolf wrote: Am 01.04.2015 um 18:16 hat John Snow geschrieben: Kevin, what's the correct behavior for qemu-img and relative paths when creating a new qcow2 file? Example: (in e.g. /home/qemu/build/ or anywhere not /home: ) qemu-img create -f qcow2 base.qcow2 32G qemu-img create -f qcow2 -F qcow2 -b base.qcow2 /home/overlay.qcow2 In 1.7.0., this produces a warning that the base object cannot be found (because it does not exist at that location relative to overlay.qcow2), but qemu-img will create the qcow2 for you regardless. 2.0, 2.1 and 2.2 all will create the image successfully, with no warnings. 2.3-rc1/master as they exist now will emit an error message and create no image. Since this is a change in behavior for the pending release, is this the correct/desired behavior? Part one of the answer is easy: qemu-img create should succeed if, and only if, a usable image is created. This requires that the backing file exists. So far so good. Part two is a bit harder: Should base.qcow2 be found in the current directory even if the new image is somewhere else? We must give preference to an existing base.qcow2 relative to the new image path, but if it doesn't exist, we could in theory try to find it relative to the working directory. Nack. This seems like inviting heartbreak unnecessarily. If we then find it, we have two options: Either we use that image (probably with an absolute path then?) or we print a useful error message that instructs the user how relative paths work with images. I think the latter is better because the other option feels like too much magic. Too much magic indeed. I think where ambiguity of paths is concerned, it is best to stick to one particular path and make it very explicit. In this case, if we cannot find some relative path offered by the user, an error message such as: "Hey! We can't find /absolute/path/to/../your/relative/file.qcow2" should be sufficient to clue the user in to where qemu-img is looking for this backing file. A usability bonus might be when we go to whine at the user, if the file exists relative to the PWD: "Qemu noticed a file at /your/pwd/.../your/relative/file.qcow2, but Qemu expects relative paths for backing files to be relative to the image referencing them, not your current PWD" but this is just a Deluxe Niceness. In any case, the behaviour you describe for 2.3-rc1 seems to be the best that we've had until now; 1.7.0 looks like the second best. We should probably "document" the 2.3-rc1 behaviour with a qemu-iotests case. Absolutely. Oh, and we still have a bug: If you specify an image size, qemu-img doesn't check at all whether the backing file exists. Kevin Thanks, --js
Re: [Qemu-block] [Qemu-devel] [PATCH v4 19/20] iotests: add simple incremental backup case
On 04/02/2015 10:27 AM, Stefan Hajnoczi wrote:
On Fri, Mar 20, 2015 at 03:17:02PM -0400, John Snow wrote:
Signed-off-by: John Snow
---
tests/qemu-iotests/124 | 153 +
tests/qemu-iotests/124.out | 4 +-
2 files changed, 155 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 85675ec..ce2cda7 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
[snip, snip, snip]
class TestIncrementalBackup(iotests.QMPTestCase):
def setUp(self):
@@ -73,6 +109,123 @@ class TestIncrementalBackup(iotests.QMPTestCase):
iotests.qemu_img('create', '-f', fmt, img, size)
self.files.append(img)
+
+def create_full_backup(self, drive=None):
+if drive is None:
+drive = self.drives[-1]
+
+res = self.vm.qmp('drive-backup', device=drive['id'],
+ sync='full', format=drive['fmt'],
+ target=drive['backup'])
+self.assert_qmp(res, 'return', {})
+self.wait_until_completed(drive['id'])
+self.check_full_backup(drive)
+self.files.append(drive['backup'])
+return drive['backup']
+
+
+def check_full_backup(self, drive=None):
+if drive is None:
+drive = self.drives[-1]
+self.assertTrue(iotests.compare_images(drive['file'], drive['backup']))
I think QEMU still has at least drive['file'] open? It's not safe to
access the file from another program while it is open.
The simplest solution is to terminate the VM before calling
iotests.compare_images().
Oh, that's unfortunate. I have been checking images after every creation
as a nice incremental sanity check. That will be hard to do if I have to
actually close QEMU.
My reasoning was:
(1) We're explicitly flushing after every write,
(2) We're in a qtest mode so there is no guest activity,
(3) Nobody is writing to this image by the time we call compare_images().
So it should be safe to /read/ the files while QEMU is occupied doing
nothing.
I could delay all tests until completion, but I'd lose out on the
ability to test the equivalence of any incrementals that are not their
most recent versions, unless I also start creating a lot of full
backups, but I think this starts to get messy and makes the tests hard
to follow.
Is it really that unsafe? I could add in an explicit pause/resume
barrier around the check if that would help inspire some confidence in
the test.
--js
Re: [Qemu-block] Migration sometimes fails with IDE and Qemu 2.2.1
Am 06.04.2015 um 21:02 schrieb Peter Lieven: > Am 06.04.2015 um 20:50 schrieb John Snow: >> >> On 04/06/2015 02:47 PM, Peter Lieven wrote: >>> Hi all, >>> >>> is there a known issue in Qemu 2.2.1 where IDE stalls sometimes after a >>> migration with Qemu 2.2.1? >>> The migration succeeds, but it seems that the complete I/O is hanging. This >>> happens only sometimes >>> and only with extreme old Linux Guests (SLES 10 with Kernel 2.6.16) thus >>> the IDE controller as >>> storage controller for the system disk. >>> >>> Maybe this sounds familiar to someone. >>> >>> Thank you, >>> Peter >>> >> It's news to me. > Okay, I was hoping that I just missed a patch or someone forgot to CC > qemu-stable... :-) > >> Is this a regression? > I can't say we see those vServers hang sometime after migration since we > changed the hypervisor from qemu-kvm-1.2.0 to qemu-2.2.0. > > >> Any particular workload or reproducer? > Workload is almost zero. I try to figure out if there is a way to trigger it. > > Maybe playing a role: Machine type is -M pc1.2 and we set -kvmclock as > CPU flag since kvmclock seemed to be quite buggy in 2.6.16... > > Exact cmdline is: > /usr/bin/qemu-2.2.1 -enable-kvm -M pc-1.2 -nodefaults -netdev > type=tap,id=guest2,script=no,downscript=no,ifname=tap2 -device > e1000,netdev=guest2,mac=52:54:00:ff:00:65 -drive > format=raw,file=iscsi://172.21.200.53/iqn.2001-05.com.equallogic:4-52aed6-88a7e99a4-d9e00040fdc509a3-XXX-hd0/0,if=ide,cache=writeback,aio=native > -serial null -parallel null -m 1024 -smp 2,sockets=1,cores=2,threads=1 > -monitor tcp:0:4003,server,nowait -vnc :3 -qmp tcp:0:3003,server,nowait -name > 'XXX' -boot order=c,once=dc,menu=off -drive > index=2,media=cdrom,if=ide,cache=unsafe,aio=native,readonly=on -k de > -incoming tcp:0:5003 -pidfile /var/run/qemu/vm-146.pid -mem-path /hugepages > -mem-prealloc -rtc base=utc -usb -usbdevice tablet -no-hpet -vga cirrus > -cpu qemu64,-kvmclock > > Exact kernel is: > 2.6.16.46-0.12-smp (i think this is SLES10 or sth.) > > The machine does not hang. It seems just I/O is hanging. So you can type at > the console or ping the system, but no longer login. > > Thank you, > Peter Interesting observation: Migrating the vServer again seems to fix to problem (at least in one case I could test just now). 2.6.8-24-smp is also affected. Thanks, Peter
Re: [Qemu-block] Migration sometimes fails with IDE and Qemu 2.2.1
Am 06.04.2015 um 20:50 schrieb John Snow: > > > On 04/06/2015 02:47 PM, Peter Lieven wrote: >> Hi all, >> >> is there a known issue in Qemu 2.2.1 where IDE stalls sometimes after a >> migration with Qemu 2.2.1? >> The migration succeeds, but it seems that the complete I/O is hanging. This >> happens only sometimes >> and only with extreme old Linux Guests (SLES 10 with Kernel 2.6.16) thus the >> IDE controller as >> storage controller for the system disk. >> >> Maybe this sounds familiar to someone. >> >> Thank you, >> Peter >> > > It's news to me. Okay, I was hoping that I just missed a patch or someone forgot to CC qemu-stable... :-) > > Is this a regression? I can't say we see those vServers hang sometime after migration since we changed the hypervisor from qemu-kvm-1.2.0 to qemu-2.2.0. > Any particular workload or reproducer? Workload is almost zero. I try to figure out if there is a way to trigger it. Maybe playing a role: Machine type is -M pc1.2 and we set -kvmclock as CPU flag since kvmclock seemed to be quite buggy in 2.6.16... Exact cmdline is: /usr/bin/qemu-2.2.1 -enable-kvm -M pc-1.2 -nodefaults -netdev type=tap,id=guest2,script=no,downscript=no,ifname=tap2 -device e1000,netdev=guest2,mac=52:54:00:ff:00:65 -drive format=raw,file=iscsi://172.21.200.53/iqn.2001-05.com.equallogic:4-52aed6-88a7e99a4-d9e00040fdc509a3-XXX-hd0/0,if=ide,cache=writeback,aio=native -serial null -parallel null -m 1024 -smp 2,sockets=1,cores=2,threads=1 -monitor tcp:0:4003,server,nowait -vnc :3 -qmp tcp:0:3003,server,nowait -name 'XXX' -boot order=c,once=dc,menu=off -drive index=2,media=cdrom,if=ide,cache=unsafe,aio=native,readonly=on -k de -incoming tcp:0:5003 -pidfile /var/run/qemu/vm-146.pid -mem-path /hugepages -mem-prealloc -rtc base=utc -usb -usbdevice tablet -no-hpet -vga cirrus -cpu qemu64,-kvmclock Exact kernel is: 2.6.16.46-0.12-smp (i think this is SLES10 or sth.) The machine does not hang. It seems just I/O is hanging. So you can type at the console or ping the system, but no longer login. Thank you, Peter
Re: [Qemu-block] Migration sometimes fails with IDE and Qemu 2.2.1
On 04/06/2015 02:47 PM, Peter Lieven wrote: Hi all, is there a known issue in Qemu 2.2.1 where IDE stalls sometimes after a migration with Qemu 2.2.1? The migration succeeds, but it seems that the complete I/O is hanging. This happens only sometimes and only with extreme old Linux Guests (SLES 10 with Kernel 2.6.16) thus the IDE controller as storage controller for the system disk. Maybe this sounds familiar to someone. Thank you, Peter It's news to me. Is this a regression? Any particular workload or reproducer? --js
[Qemu-block] Migration sometimes fails with IDE and Qemu 2.2.1
Hi all, is there a known issue in Qemu 2.2.1 where IDE stalls sometimes after a migration with Qemu 2.2.1? The migration succeeds, but it seems that the complete I/O is hanging. This happens only sometimes and only with extreme old Linux Guests (SLES 10 with Kernel 2.6.16) thus the IDE controller as storage controller for the system disk. Maybe this sounds familiar to someone. Thank you, Peter
Re: [Qemu-block] block-commit & dropping privs
02.04.2015 16:19, Kevin Wolf wrote: > Am 02.04.2015 um 14:04 hat Michael Tokarev geschrieben: >> 02.04.2015 14:24, Kevin Wolf wrote: >> [] But overall, I think qemu-system should not modify backing file name in this case. >>> >>> So you would leave the backing file with the data that you just >>> committed down one level in your backing file chain? Wouldn't that >>> defeat the whole purpose of committing? >> >> Um. I don't think we understood each other. >> >> I experimented with the "non-live" HMP commit command. This >> one effectively empties everything in the overlay file, >> propagating it to the backing file, but keeps the (now >> empty) overlay. So from the stacking perspective nothing >> has changed. Yet, together with with propagation, it also >> modifies the overlay file headers and writes a new name >> of the backing file -- the one it currently uses, which, >> in my case, is virtual /dev/fdset/foo. It should keep >> the original file name in there, such as win.raw, unless >> explicitly asked to write a different name there. >> >> If the stack chain were to be modified by commit command, >> yes, the new name should be recorded ofcourse, such as >> after rebase. But since stack chain is not modified, >> filename should not be modified either. > > For the record, we discussed this on IRC: > > Yes, we did talk past each other because HMP commit isn't supposed to > touch the backing file name at all, so I didn't expect such behaviour, > yet Michael saw it. > > The reason is a bug in qcow2_update_header(). Instead of rewriting the > same value as is already in the image, it writes bs->backing_file to the > image. This was always the same as long as you couldn't override the > backing file name on the command line or in blockdev-add, but now it's > obviously wrong. > > It would also rewrite the backing file name on other occasions such as > marking the image dirty with lazy_refcounts=on (i.e. on the first > write request). Kevin, did you have a chance to fix this prob (for 2.3)? 2.1 version does the right thing here. When performing commit, does qemu mark the areas in the overlay file as free after writing contents to the backing file, or will these areas be written again by a subsequent commit? Somehow it smells like each next commit writes more and more data and completes in more and more time. >>> >>> With qcow2 and qcow, the committed data is discarded with HMP 'commit'. >>> Other image formats keep the copy. >> >> Hm. It is discarded, but the file isn't shrinked. With "non-live" >> commit I don't see a reason why it can't be shrinked too? > > This would be a bug as well, but Michael double-checked and it does > shrink in fact. Actually it WAS a bug. Initially I tested with 2.1 version, and there, neither `commit' HMP command nor `qemu-img commit' command shrinks the overlay image, so each new commit re-writes both the new data AND the old data, and never shrinks. With 2.2 version it works as expected, except of the above problem with rewriting the base file name. Thanks, /mjt
