[libvirt] [PATCH 0/3] qemu: Fix formatting of (some) unsigned long long values to JSON (blockdev-add saga)
Currently, libvirt would regress to computer-middle-ages by not being able to use more than 4GiB images for snapshots without corrupting data. Fix it by using the proper formatting modifier. Peter Krempa (3): qemu: block: Use correct type when creating image size JSON entries tests: qemublock: Use bigger numbers as dummy capacity/physical qemu: monitor: Fix formatting of 'offset' in qemuMonitorJSONSaveMemory src/qemu/qemu_block.c | 12 ++++++------ src/qemu/qemu_monitor_json.c | 2 +- tests/qemublocktest.c | 4 ++-- .../qemublocktestdata/imagecreate/luks-encopts.json | 4 ++-- tests/qemublocktestdata/imagecreate/luks-noopts.json | 4 ++-- .../imagecreate/network-gluster-qcow2.json | 4 ++-- .../imagecreate/network-rbd-qcow2.json | 4 ++-- .../imagecreate/network-sheepdog-qcow2.json | 4 ++-- .../imagecreate/network-ssh-qcow2.json | 4 ++-- .../imagecreate/qcow2-backing-luks.json | 4 ++-- .../imagecreate/qcow2-backing-raw-nbd.json | 4 ++-- .../imagecreate/qcow2-backing-raw.json | 4 ++-- .../imagecreate/qcow2-luks-encopts-backing.json | 4 ++-- .../imagecreate/qcow2-luks-encopts.json | 4 ++-- .../imagecreate/qcow2-luks-noopts.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2.json | 4 ++-- tests/qemublocktestdata/imagecreate/raw.json | 2 +- 17 files changed, 36 insertions(+), 36 deletions(-) -- 2.21.0
The 'u' modifier creates a unsigned int JSON attribute but the disk size and capacity fields are unsigned long long. If the size of the created image would be more than 4GiB we'd overflow and create sub-4G image. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 66b1d116d8..e33aad4458 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -2034,7 +2034,7 @@ qemuBlockStorageSourceCreateGetFormatPropsGeneric(virStorageSourcePtr src, if (virJSONValueObjectCreate(&props, "s:driver", driver, "s:file", src->nodestorage, - "u:size", src->capacity, + "U:size", src->capacity, NULL) < 0) return -1; @@ -2100,7 +2100,7 @@ qemuBlockStorageSourceCreateGetFormatPropsLUKS(virStorageSourcePtr src, if (virJSONValueObjectAdd(luksprops, "s:driver", "luks", "s:file", src->nodestorage, - "u:size", src->capacity, + "U:size", src->capacity, NULL) < 0) return -1; @@ -2153,7 +2153,7 @@ qemuBlockStorageSourceCreateGetFormatPropsQcow2(virStorageSourcePtr src, if (virJSONValueObjectCreate(&qcow2props, "s:driver", "qcow2", "s:file", src->nodestorage, - "u:size", src->capacity, + "U:size", src->capacity, "S:version", qcow2version, NULL) < 0) return -1; @@ -2177,7 +2177,7 @@ qemuBlockStorageSourceCreateGetFormatPropsQcow(virStorageSourcePtr src, if (virJSONValueObjectCreate(&qcowprops, "s:driver", "qcow", "s:file", src->nodestorage, - "u:size", src->capacity, + "U:size", src->capacity, NULL) < 0) return -1; @@ -2200,7 +2200,7 @@ qemuBlockStorageSourceCreateGetFormatPropsQed(virStorageSourcePtr src, if (virJSONValueObjectCreate(&qedprops, "s:driver", "qed", "s:file", src->nodestorage, - "u:size", src->capacity, + "U:size", src->capacity, NULL) < 0) return -1; @@ -2373,7 +2373,7 @@ qemuBlockStorageSourceCreateGetStorageProps(virStorageSourcePtr src, "s:driver", driver, "S:filename", filename, "A:location", &location, - "u:size", src->physical, + "U:size", src->physical, NULL) < 0) return -1; -- 2.21.0
On Fri, Aug 30, 2019 at 04:45:03PM +0200, Peter Krempa wrote:
The 'u' modifier creates a unsigned int JSON attribute but the disk size
an unsigned
and capacity fields are unsigned long long. If the size of the created image would be more than 4GiB we'd overflow and create sub-4G image.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
Depending on how broken things get, qemuBlockStorageSourceGetGlusterProps might need a similar change ;) Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Actually test that the full range is available. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 4 ++-- tests/qemublocktestdata/imagecreate/luks-encopts.json | 4 ++-- tests/qemublocktestdata/imagecreate/luks-noopts.json | 4 ++-- .../qemublocktestdata/imagecreate/network-gluster-qcow2.json | 4 ++-- tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json | 4 ++-- .../qemublocktestdata/imagecreate/network-sheepdog-qcow2.json | 4 ++-- tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2-backing-luks.json | 4 ++-- .../qemublocktestdata/imagecreate/qcow2-backing-raw-nbd.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2-backing-raw.json | 4 ++-- .../imagecreate/qcow2-luks-encopts-backing.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2.json | 4 ++-- tests/qemublocktestdata/imagecreate/raw.json | 2 +- 15 files changed, 29 insertions(+), 29 deletions(-) diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 0bc4b65449..1bf72a4615 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -420,8 +420,8 @@ testQemuImageCreate(const void *opaque) return -1; /* fake some sizes */ - src->capacity = 1337; - src->physical = 42; + src->capacity = UINT_MAX * 2ULL; + src->physical = UINT_MAX + 1ULL; if (qemuDomainValidateStorageSource(src, data->qemuCaps) < 0) return -1; diff --git a/tests/qemublocktestdata/imagecreate/luks-encopts.json b/tests/qemublocktestdata/imagecreate/luks-encopts.json index f065ad89a7..c5ca863f5b 100644 --- a/tests/qemublocktestdata/imagecreate/luks-encopts.json +++ b/tests/qemublocktestdata/imagecreate/luks-encopts.json @@ -2,7 +2,7 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.img", - "size": 42 + "size": 4294967296 } format: @@ -15,5 +15,5 @@ format: "ivgen-hash-alg": "sha256", "driver": "luks", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337 + "size": 8589934590 } diff --git a/tests/qemublocktestdata/imagecreate/luks-noopts.json b/tests/qemublocktestdata/imagecreate/luks-noopts.json index 1ea1948119..8a0944151d 100644 --- a/tests/qemublocktestdata/imagecreate/luks-noopts.json +++ b/tests/qemublocktestdata/imagecreate/luks-noopts.json @@ -2,7 +2,7 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.img", - "size": 42 + "size": 4294967296 } format: @@ -10,5 +10,5 @@ format: "key-secret": "0123456789ABCDEF0123456789ABCDE-encalias", "driver": "luks", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337 + "size": 8589934590 } diff --git a/tests/qemublocktestdata/imagecreate/network-gluster-qcow2.json b/tests/qemublocktestdata/imagecreate/network-gluster-qcow2.json index aee7bfd401..3e35beb088 100644 --- a/tests/qemublocktestdata/imagecreate/network-gluster-qcow2.json +++ b/tests/qemublocktestdata/imagecreate/network-gluster-qcow2.json @@ -17,12 +17,12 @@ protocol: } ] }, - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337 + "size": 8589934590 } diff --git a/tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json b/tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json index 56d9c0f1ed..67e2679dae 100644 --- a/tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json +++ b/tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json @@ -15,12 +15,12 @@ protocol: } ] }, - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337 + "size": 8589934590 } diff --git a/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json b/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json index b7272625a2..d86bef6bc8 100644 --- a/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json +++ b/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json @@ -9,12 +9,12 @@ protocol: }, "vdi": "asdf/i.qcow2" }, - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337 + "size": 8589934590 } diff --git a/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json b/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json index 31416ed4fc..d58054c081 100644 --- a/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json +++ b/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json @@ -8,12 +8,12 @@ protocol: "port": "1234" } }, - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337 + "size": 8589934590 } diff --git a/tests/qemublocktestdata/imagecreate/qcow2-backing-luks.json b/tests/qemublocktestdata/imagecreate/qcow2-backing-luks.json index 63ba35dc79..5f9a800c6c 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2-backing-luks.json +++ b/tests/qemublocktestdata/imagecreate/qcow2-backing-luks.json @@ -2,14 +2,14 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.qcow2", - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337, + "size": 8589934590, "backing-file": "/var/lib/libvirt/images/i.img", "backing-fmt": "luks" } diff --git a/tests/qemublocktestdata/imagecreate/qcow2-backing-raw-nbd.json b/tests/qemublocktestdata/imagecreate/qcow2-backing-raw-nbd.json index b9d1d97302..c10ab98c8a 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2-backing-raw-nbd.json +++ b/tests/qemublocktestdata/imagecreate/qcow2-backing-raw-nbd.json @@ -2,14 +2,14 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.qcow2", - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337, + "size": 8589934590, "backing-file": "nbd://example.com:1234", "backing-fmt": "raw" } diff --git a/tests/qemublocktestdata/imagecreate/qcow2-backing-raw.json b/tests/qemublocktestdata/imagecreate/qcow2-backing-raw.json index 8176c8dadd..eb9fb413f6 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2-backing-raw.json +++ b/tests/qemublocktestdata/imagecreate/qcow2-backing-raw.json @@ -2,14 +2,14 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.qcow2", - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337, + "size": 8589934590, "backing-file": "/var/lib/libvirt/images/i.img", "backing-fmt": "raw" } diff --git a/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts-backing.json b/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts-backing.json index a57617dfac..641b5e04c9 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts-backing.json +++ b/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts-backing.json @@ -2,14 +2,14 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.qcow2", - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337, + "size": 8589934590, "backing-file": "/var/lib/libvirt/images/i.qcow2", "backing-fmt": "qcow2", "encrypt": { diff --git a/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.json b/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.json index 8796726fcb..28c85ec90b 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.json +++ b/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.json @@ -2,14 +2,14 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.qcow2", - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337, + "size": 8589934590, "encrypt": { "key-secret": "0123456789ABCDEF0123456789ABCDE-encalias", "cipher-alg": "serpent-256", diff --git a/tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.json b/tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.json index f9caaee6bb..b5063a846d 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.json +++ b/tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.json @@ -2,14 +2,14 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.qcow2", - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337, + "size": 8589934590, "encrypt": { "key-secret": "0123456789ABCDEF0123456789ABCDE-encalias", "format": "luks" diff --git a/tests/qemublocktestdata/imagecreate/qcow2.json b/tests/qemublocktestdata/imagecreate/qcow2.json index 7142cf67b6..732763b763 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2.json +++ b/tests/qemublocktestdata/imagecreate/qcow2.json @@ -2,12 +2,12 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.qcow2", - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337 + "size": 8589934590 } diff --git a/tests/qemublocktestdata/imagecreate/raw.json b/tests/qemublocktestdata/imagecreate/raw.json index 06abb25ab9..89a6c2d237 100644 --- a/tests/qemublocktestdata/imagecreate/raw.json +++ b/tests/qemublocktestdata/imagecreate/raw.json @@ -2,7 +2,7 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.img", - "size": 42 + "size": 4294967296 } format: -- 2.21.0
On Fri, Aug 30, 2019 at 04:45:04PM +0200, Peter Krempa wrote:
Actually test that the full range is available.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 4 ++-- tests/qemublocktestdata/imagecreate/luks-encopts.json | 4 ++-- tests/qemublocktestdata/imagecreate/luks-noopts.json | 4 ++-- .../qemublocktestdata/imagecreate/network-gluster-qcow2.json | 4 ++-- tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json | 4 ++-- .../qemublocktestdata/imagecreate/network-sheepdog-qcow2.json | 4 ++-- tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2-backing-luks.json | 4 ++-- .../qemublocktestdata/imagecreate/qcow2-backing-raw-nbd.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2-backing-raw.json | 4 ++-- .../imagecreate/qcow2-luks-encopts-backing.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2.json | 4 ++-- tests/qemublocktestdata/imagecreate/raw.json | 2 +- 15 files changed, 29 insertions(+), 29 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Peter Krempa writes:
Actually test that the full range is available.
[...]
return -1; diff --git a/tests/qemublocktestdata/imagecreate/luks-encopts.json b/tests/qemublocktestdata/imagecreate/luks-encopts.json index f065ad89a7..c5ca863f5b 100644 --- a/tests/qemublocktestdata/imagecreate/luks-encopts.json +++ b/tests/qemublocktestdata/imagecreate/luks-encopts.json @@ -2,7 +2,7 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.img", - "size": 42 + "size": 4294967296 }
Does any of these tests actually require that much space to run? If so, what is the impact on QE teams? -- Cheers, Christophe de Dinechin (IRC c3d)
On Fri, Aug 30, 2019 at 05:30:35PM +0200, Christophe de Dinechin wrote:
Peter Krempa writes:
Actually test that the full range is available.
[...]
return -1; diff --git a/tests/qemublocktestdata/imagecreate/luks-encopts.json b/tests/qemublocktestdata/imagecreate/luks-encopts.json index f065ad89a7..c5ca863f5b 100644 --- a/tests/qemublocktestdata/imagecreate/luks-encopts.json +++ b/tests/qemublocktestdata/imagecreate/luks-encopts.json @@ -2,7 +2,7 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.img", - "size": 42 + "size": 4294967296 }
Does any of these tests actually require that much space to run?
No, parsing a four hundred byte json file does not really take much space. Jano
If so, what is the impact on QE teams?
-- Cheers, Christophe de Dinechin (IRC c3d)
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
The offset is unsigned long long thus 'U' must be used. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index da1e89dded..e4404f0199 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3123,7 +3123,7 @@ static int qemuMonitorJSONSaveMemory(qemuMonitorPtr mon, int ret = -1; virJSONValuePtr cmd = qemuMonitorJSONMakeCommand(cmdtype, "U:val", offset, - "u:size", length, + "U:size", length, "s:filename", path, NULL); virJSONValuePtr reply = NULL; -- 2.21.0
On Fri, Aug 30, 2019 at 04:45:05PM +0200, Peter Krempa wrote:
The offset is unsigned long long thus 'U' must be used.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (3)
-
Christophe de Dinechin -
Ján Tomko -
Peter Krempa