12:50 a.m.
Regression is introduced by commit e30297b0. After that, it will
report "Cannot fill file" error with following xml file:
<volume>
<name>virtinst-vmlinuz-xen.tP1NHh</name>
<capacity>4343792</capacity>
<allocation>0</allocation>
<target>
<format type="raw"/>
<nocow/>
</target>
</volume>
Because of this, installing xen pv guest with virt-manager fails.
Reason is: posix_fallocate(int fd, off_t offset, off_t len) will
return EINVAL when len is equal to 0. So, this patch adds a check
before calling safezero().
Signed-off-by: Chunyan Liu <cyliu(a)suse.com>
---
src/storage/storage_backend.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index db49739..2a265af 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -455,7 +455,7 @@ createRawFile(int fd, virStorageVolDefPtr vol,
pos = inputvol->target.capacity - remain;
}
- if (need_alloc) {
+ if (need_alloc && (vol->target.allocation > pos)) {
if (safezero(fd, pos, vol->target.allocation - pos) < 0) {
ret = -errno;
virReportSystemError(errno, _("cannot fill file '%s'"),
--
2.1.4
4:47 a.m.
On Mon, Aug 24, 2015 at 01:50:41PM +0800, Chunyan Liu wrote:
Regression is introduced by commit e30297b0. After that, it will
report "Cannot fill file" error with following xml file:
<volume>
<name>virtinst-vmlinuz-xen.tP1NHh</name>
<capacity>4343792</capacity>
<allocation>0</allocation>
<target>
<format type="raw"/>
<nocow/>
</target>
</volume>
Because of this, installing xen pv guest with virt-manager fails.
Reason is: posix_fallocate(int fd, off_t offset, off_t len) will
return EINVAL when len is equal to 0. So, this patch adds a check
before calling safezero().
Oh, god that you've found out the root cause, although the same patch
is already on the list:
https://www.redhat.com/archives/libvir-list/2015-August/msg00808.html
Although it would be nice if it had the root cause (or at least the
causing commit) in its commit message as well. Cc'ing the author of
the previous patch in case he hasn't pushed yet.
Martin
5:55 a.m.
New subject: [libvirt] [PATCH v2] storage: only run safezero if allocation is > 0
While a zero allocation in safezero should be fine it isn't when we use
posix_fallocate which returns EINVAL on a zero allocation.
While we could skip the zero allocation in safezero_posix_fallocate it's
an optimization to do it for all allocations.
This fixes vm installation via virtinst for me which otherwise aborts
like:
Starting install...
Retrieving file linux... | 5.9 MB 00:01 ...
Retrieving file initrd.gz... | 29 MB 00:07 ...
ERROR Couldn't create storage volume 'virtinst-linux.sBgds4':
'cannot fill file '/var/lib/libvirt/boot/virtinst-linux.sBgds4': Invalid
argument'
The error was introduced by e30297b0 as spotted by Chunyan Liu
---
Only the commit message got adjusted.
src/storage/storage_backend.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index db49739..0418473 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -455,7 +455,7 @@ createRawFile(int fd, virStorageVolDefPtr vol,
pos = inputvol->target.capacity - remain;
}
- if (need_alloc) {
+ if (need_alloc && (vol->target.allocation - pos > 0)) {
if (safezero(fd, pos, vol->target.allocation - pos) < 0) {
ret = -errno;
virReportSystemError(errno, _("cannot fill file '%s'"),
--
2.1.4
6:27 a.m.
New subject: [libvirt] [PATCH v2] storage: only run safezero if allocation is > 0
On Mon, Aug 24, 2015 at 12:55:22PM +0200, Guido Günther wrote:
While a zero allocation in safezero should be fine it isn't when
we use
posix_fallocate which returns EINVAL on a zero allocation.
While we could skip the zero allocation in safezero_posix_fallocate it's
an optimization to do it for all allocations.
This fixes vm installation via virtinst for me which otherwise aborts
like:
Starting install...
Retrieving file linux... | 5.9 MB 00:01 ...
Retrieving file initrd.gz... | 29 MB 00:07 ...
ERROR Couldn't create storage volume 'virtinst-linux.sBgds4':
'cannot fill file '/var/lib/libvirt/boot/virtinst-linux.sBgds4': Invalid
argument'
The error was introduced by e30297b0 as spotted by Chunyan Liu
---
Only the commit message got adjusted.
ACK, that's a possible one.
src/storage/storage_backend.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index db49739..0418473 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -455,7 +455,7 @@ createRawFile(int fd, virStorageVolDefPtr vol,
pos = inputvol->target.capacity - remain;
}
- if (need_alloc) {
+ if (need_alloc && (vol->target.allocation - pos > 0)) {
if (safezero(fd, pos, vol->target.allocation - pos) < 0) {
ret = -errno;
virReportSystemError(errno, _("cannot fill file '%s'"),
--
2.1.4
6:53 a.m.
New subject: [libvirt] [PATCH v2] storage: only run safezero if allocation is > 0
Hi,
On Mon, Aug 24, 2015 at 01:27:42PM +0200, Martin Kletzander wrote:
On Mon, Aug 24, 2015 at 12:55:22PM +0200, Guido Günther wrote:
>While a zero allocation in safezero should be fine it isn't when we use
>posix_fallocate which returns EINVAL on a zero allocation.
>
>While we could skip the zero allocation in safezero_posix_fallocate it's
>an optimization to do it for all allocations.
>
>This fixes vm installation via virtinst for me which otherwise aborts
>like:
>
> Starting install...
> Retrieving file linux... | 5.9 MB 00:01 ...
> Retrieving file initrd.gz... | 29 MB 00:07 ...
> ERROR Couldn't create storage volume 'virtinst-linux.sBgds4':
'cannot fill file '/var/lib/libvirt/boot/virtinst-linux.sBgds4': Invalid
argument'
>
>The error was introduced by e30297b0 as spotted by Chunyan Liu
>---
>Only the commit message got adjusted.
>
ACK, that's a possible one.
Pushed now. Thanks!
-- Guido
>src/storage/storage_backend.c | 2 +-
>1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
>index db49739..0418473 100644
>--- a/src/storage/storage_backend.c
>+++ b/src/storage/storage_backend.c
>@@ -455,7 +455,7 @@ createRawFile(int fd, virStorageVolDefPtr vol,
> pos = inputvol->target.capacity - remain;
> }
>
>- if (need_alloc) {
>+ if (need_alloc && (vol->target.allocation - pos > 0)) {
> if (safezero(fd, pos, vol->target.allocation - pos) < 0) {
> ret = -errno;
> virReportSystemError(errno, _("cannot fill file
'%s'"),
>--
>2.1.4
8:17 p.m.
>> On 8/24/2015 at 05:47 PM, in message
<20150824094707.GD22373@wheatley>, Martin
Kletzander
<mkletzan(a)redhat.com> wrote:
On Mon, Aug 24, 2015 at 01:50:41PM +0800, Chunyan Liu wrote:
>Regression is introduced by commit e30297b0. After that, it will
>report "Cannot fill file" error with following xml file:
><volume>
> <name>virtinst-vmlinuz-xen.tP1NHh</name>
> <capacity>4343792</capacity>
> <allocation>0</allocation>
> <target>
> <format type="raw"/>
> <nocow/>
> </target>
></volume>
>Because of this, installing xen pv guest with virt-manager fails.
>
>Reason is: posix_fallocate(int fd, off_t offset, off_t len) will
>return EINVAL when len is equal to 0. So, this patch adds a check
>before calling safezero().
>
Oh, god that you've found out the root cause, although the same patch
is already on the list:
https://www.redhat.com/archives/libvir-list/2015-August/msg00808.html
Oh yes, it's the same patch. Sorry for not reading the message in time. Really
coincidental sending same patch from different person one after another :)
Chunyan
Although it would be nice if it had the root cause (or at least the
causing commit) in its commit message as well. Cc'ing the author of
the previous patch in case he hasn't pushed yet.
Martin
3380
days inactive
3381
days old
5 comments
4 participants
participants (4)
-
Chun Yan Liu -
Chunyan Liu
-
Guido Günther -
Martin Kletzander