Re: [pve-devel] [PATCH pve-qemu 1/1] add block-commit-replaces option patch

["DERUMIER, Alexandre via pve-devel" <pve-devel@lists.proxmox.com>]

[-- Attachment #1: Type: message/rfc822, Size: 25189 bytes --]

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.ebner@proxmox.com" <f.ebner@proxmox.com>
Subject: Re: [pve-devel] [PATCH pve-qemu 1/1] add block-commit-replaces option patch
Date: Tue, 6 May 2025 13:35:55 +0000
Message-ID: <0f39ca25e85cc9d859d515203262bc68437f518d.camel@groupe-cyllene.com>

Ok, I have tested it's working !

>>But setting "top-node" being the throttle node like here:
btw, in your example, top-node=node0 is not the throttle node

"--blockdev
throttle,node-name=drive-scsi0,throttle-group=thrgr0,file.driver=qcow
2,file.node-name=node0"

This is the fmt blocknode under the throttle node.

but yes, it's working.
I'm pretty sure that it was not working last year. (the blocknode after
commit was not attached to any node anymore). Maybe it has been fixed
in qemu, I don't know ¯\_(ツ)_/¯


So, no need for this patch !



qemu-server codefix : (I had old unused code too when I was using
commit of intermediate snapshot, and top-node was working because it
was not behind the throttle group)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index aff430df..95b31258 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4655,20 +4655,12 @@ sub blockdev_commit {
     my $jobs = {};
     my $opts = { 'job-id' => $job_id, device => $deviceid };
 
-    my $complete = undef;
-    if ($src_snap && $src_snap ne 'current') {
-       $complete = 'auto';
-       $opts->{'top-node'} = $src_fmt_blockdev->{'node-name'};
-       $opts->{'base-node'} = $target_fmt_blockdev->{'node-name'};
-    } else {
-       $complete = 'complete';
-       $opts->{'base-node'} = $target_fmt_blockdev->{'node-name'};
-       $opts->{replaces} = $src_fmt_blockdev->{'node-name'};
-    }
+    $opts->{'base-node'} = $target_fmt_blockdev->{'node-name'};
+    $opts->{'top-node'} = $src_fmt_blockdev->{'node-name'};
 
     mon_cmd($vmid, "block-commit", %$opts);
     $jobs->{$job_id} = {};
-    qemu_drive_mirror_monitor($vmid, undef, $jobs, $complete, 0,
'commit');
+    qemu_drive_mirror_monitor($vmid, undef, $jobs, 'complete', 0,
'commit');


-------- Message initial --------
De: Fiona Ebner <f.ebner@proxmox.com>
À: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Cc: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
Objet: Re: [pve-devel] [PATCH pve-qemu 1/1] add block-commit-replaces
option patch
Date: 06/05/2025 11:00:07

Hi,

Am 22.04.25 um 13:51 schrieb Alexandre Derumier via pve-devel:
> This is needed for external snapshot live commit,
> when the top blocknode is not the fmt-node.
> (in our case, the throttle-group node is the topnode)
> 
> Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-
> cyllene.com>

I looked into this again and maybe we can avoid this patch. I think all
you need is to specify the 'top-node' when issuing the commit command.

Without specifying "top-node":

> [I] root@pve8a1 ~# cat block-commit.sh
> #!/bin/bash
> rm -f /tmp/backing.qcow2
> rm -f /tmp/top.qcow2
> qemu-img create /tmp/backing.qcow2 -f qcow2 64M
> qemu-img create /tmp/top.qcow2 -f qcow2 64M
> qemu-system-x86_64 --qmp stdio \
> --nodefaults \
> --object throttle-group,id=thrgr0 \
> --blockdev qcow2,node-
> name=backing0,file.driver=file,file.filename=/tmp/backing.qcow2 \
> --blockdev throttle,node-name=drive-scsi0,throttle-
> group=thrgr0,file.driver=qcow2,file.node-
> name=node0,file.file.driver=file,file.file.filename=/tmp/top.qcow2,fi
> le.backing=backing0 \
> --device 'virtio-scsi-pci,id=virtioscsi0,bus=pci.0,addr=0x2' \
> --device 'scsi-hd,bus=virtioscsi0.0,channel=0,scsi-
> id=0,lun=0,drive=drive-scsi0,id=scsi0' \
> <<EOF
> {"execute": "qmp_capabilities"}
> {"execute": "query-block"}
> {"execute": "block-commit", "arguments": { "device": "drive-scsi0",
> "base-node": "backing0", "job-id": "commit0" } }
> {"execute": "block-job-complete", "arguments": {"device": "commit0" }
> }
> {"execute": "query-block"}
> {"execute": "quit"}
> EOF

The result will be:

> {
>   "return": [
>     {
>       "io-status": "ok",
>       "device": "",
>       "locked": false,
>       "removable": false,
>       "inserted": {
>         "iops_rd": 0,
>         "detect_zeroes": "off",
>         "image": {
>           "virtual-size": 67108864,
>           "filename": "/tmp/backing.qcow2",
>           "cluster-size": 65536,
>           "format": "qcow2",
>           "actual-size": 200704,
>           "format-specific": {
>             "type": "qcow2",
>             "data": {
>               "compat": "1.1",
>               "compression-type": "zlib",
>               "lazy-refcounts": false,
>               "refcount-bits": 16,
>               "corrupt": false,
>               "extended-l2": false
>             }
>           },
>           "dirty-flag": false
>         },
>         "iops_wr": 0,
>         "ro": false,
>         "node-name": "backing0",
>         "backing_file_depth": 0,
>         "drv": "qcow2",
>         "iops": 0,
>         "bps_wr": 0,
>         "write_threshold": 0,
>         "encrypted": false,
>         "bps": 0,
>         "bps_rd": 0,
>         "cache": {
>           "no-flush": false,
>           "direct": false,
>           "writeback": true
>         },
>         "file": "/tmp/backing.qcow2"
>       },
>       "qdev": "scsi0",
>       "type": "unknown"
>     }
>   ]
> }

So no throttle group, bad.

But setting "top-node" being the throttle node like here:

> #!/bin/bash
> rm -f /tmp/backing.qcow2
> rm -f /tmp/top.qcow2
> qemu-img create /tmp/backing.qcow2 -f qcow2 64M
> qemu-img create /tmp/top.qcow2 -f qcow2 64M
> qemu-system-x86_64 --qmp stdio \
> --nodefaults \
> --object throttle-group,id=thrgr0 \
> --blockdev qcow2,node-
> name=backing0,file.driver=file,file.filename=/tmp/backing.qcow2 \
> --blockdev throttle,node-name=drive-scsi0,throttle-
> group=thrgr0,file.driver=qcow2,file.node-
> name=node0,file.file.driver=file,file.file.filename=/tmp/top.qcow2,fi
> le.backing=backing0 \
> --device 'virtio-scsi-pci,id=virtioscsi0,bus=pci.0,addr=0x2' \
> --device 'scsi-hd,bus=virtioscsi0.0,channel=0,scsi-
> id=0,lun=0,drive=drive-scsi0,id=scsi0' \
> <<EOF
> {"execute": "qmp_capabilities"}
> {"execute": "query-block"}
> {"execute": "block-commit", "arguments": { "device": "drive-scsi0",
> "top-node": "node0", "base-node": "backing0", "job-id": "commit0" } }
> {"execute": "block-job-complete", "arguments": {"device": "commit0" }
> }
> {"execute": "query-block"}
> {"execute": "quit"}
> EOF

The result will be:

> {
>   "return": [
>     {
>       "io-status": "ok",
>       "device": "",
>       "locked": false,
>       "removable": false,
>       "inserted": {
>         "iops_rd": 0,
>         "detect_zeroes": "off",
>         "image": {
>           "backing-image": {
>             "virtual-size": 67108864,
>             "filename": "/tmp/backing.qcow2",
>             "cluster-size": 65536,
>             "format": "qcow2",
>             "actual-size": 200704,
>             "format-specific": {
>               "type": "qcow2",
>               "data": {
>                 "compat": "1.1",
>                 "compression-type": "zlib",
>                 "lazy-refcounts": false,
>                 "refcount-bits": 16,
>                 "corrupt": false,
>                 "extended-l2": false
>               }
>             },
>             "dirty-flag": false
>           },
>           "virtual-size": 67108864,
>           "filename": "json:{\"throttle-group\": \"thrgr0\",
> \"driver\": \"throttle\", \"file\": {\"driver\": \"qcow2\", \"file\":
> {\"driver\": \"file\", \"filename\": \"/tmp/backing.qcow2\"}}}",
>           "cluster-size": 65536,
>           "format": "throttle",
>           "actual-size": 200704,
>           "dirty-flag": false
>         },
>         "iops_wr": 0,
>         "ro": false,
>         "node-name": "drive-scsi0",
>         "backing_file_depth": 1,
>         "drv": "throttle",
>         "iops": 0,
>         "bps_wr": 0,
>         "write_threshold": 0,
>         "encrypted": false,
>         "bps": 0,
>         "bps_rd": 0,
>         "cache": {
>           "no-flush": false,
>           "direct": false,
>           "writeback": true
>         },
>         "file": "json:{\"throttle-group\": \"thrgr0\", \"driver\":
> \"throttle\", \"file\": {\"driver\": \"qcow2\", \"file\":
> {\"driver\": \"file\", \"filename\": \"/tmp/backing.qcow2\"}}}"
>       },
>       "qdev": "scsi0",
>       "type": "unknown"
>     }
>   ]
> }

I hope this is what you want?

Probably QEMU does not consider throttle groups when automatically
determining the top-node, so we need to specify it explicitly.

Best Regards,
Fiona



[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel