xen-unstable build failure in tools/blktap/drivers for block-qcow2.c

xen-unstable build failure in tools/blktap/drivers for block-qcow2.c

[Srinivas Maturi]

We are seeing build failure of xen-unstable for the last couple of 
changesets,
in tools/blktap/drivers for block-qcow2.c

This seems to be because of the fact that the compile line
uses these flags, -Werror -Wno-unused, treating warnings as errors
and aborting the compilation.

The Makefile in tools/blktap/drivers has these CFLAGS set:
CFLAGS += -Werror
CFLAGS += -Wno-unused

gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions 
-fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -m64 
-fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes 
-Wno-unused-value -Wdeclaration-after-statement -m64 
-fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes 
-Wno-unused-value -Wdeclaration-after-statement -m64 
-fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes 
-Wno-unused-value -Wdeclaration-after-statement -D__XEN_TOOLS__ 
-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -m64 -fno-strict-aliasing 
-std=gnu99 -Wall -Wstrict-prototypes -Wno-unused-value 
-Wdeclaration-after-statement -D__XEN_TOOLS__ -D_LARGEFILE_SOURCE 
-D_LARGEFILE64_SOURCE -m64 -fno-strict-aliasing -std=gnu99 -Wall 
-Wstrict-prototypes -Wno-unused-value -Wdeclaration-after-statement 
-D__XEN_TOOLS__ -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -m64 
-fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes 
-Wno-unused-value -Wdeclaration-after-statement -D__XEN_TOOLS__ 
-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -Werror -Wno-unused 
-I../../../tools/libxc -I../../../tools/include -I ../../libaio/src -I.. 
-I../lib -I../../../tools/include -I. -I../../xenstore -D_GNU_SOURCE 
-Wp,-MD,.block-qcow2.o.d -c -o block-qcow2.o block-qcow2.c
cc1: warnings being treated as errors
block-qcow2.c: In function ‘qcow2_create’:
block-qcow2.c:1312: warning: ignoring return value of ‘write’, declared 
with attribute warn_unused_result
block-qcow2.c:1314: warning: ignoring return value of ‘write’, declared 
with attribute warn_unused_result
block-qcow2.c:1319: warning: ignoring return value of ‘write’, declared 
with attribute warn_unused_result
block-qcow2.c:1322: warning: ignoring return value of ‘write’, declared 
with attribute warn_unused_result
block-qcow2.c:1325: warning: ignoring return value of ‘write’, declared 
with attribute warn_unused_result
block-qcow2.c: In function ‘qcow_write_compressed’:
block-qcow2.c:1354: warning: ignoring return value of ‘ftruncate’, 
declared with attribute warn_unused_result
.....
.....


Thanks,
Maturi

Re: xen-unstable build failure in tools/blktap/drivers for block-qcow2.c

[Keir Fraser]

Which version of gcc are you using? It works for me with 3.4.4 and 4.1.1.

Properly speaking we should check those write() return values anyway. If we
don't want to deviate from the qemu original sources that much then we could
special-case CFLAGS to remove -Wno-unused for just that file.

 -- Keir

On 27/2/08 20:08, "Srinivas Maturi" <srinivas.maturi@oracle.com> wrote:

> 
> We are seeing build failure of xen-unstable for the last couple of
> changesets,
> in tools/blktap/drivers for block-qcow2.c
> 
> This seems to be because of the fact that the compile line
> uses these flags, -Werror -Wno-unused, treating warnings as errors
> and aborting the compilation.
> 
> The Makefile in tools/blktap/drivers has these CFLAGS set:
> CFLAGS += -Werror
> CFLAGS += -Wno-unused
> 
> gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
> -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -m64
> -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes
> -Wno-unused-value -Wdeclaration-after-statement -m64
> -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes
> -Wno-unused-value -Wdeclaration-after-statement -m64
> -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes
> -Wno-unused-value -Wdeclaration-after-statement -D__XEN_TOOLS__
> -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -m64 -fno-strict-aliasing
> -std=gnu99 -Wall -Wstrict-prototypes -Wno-unused-value
> -Wdeclaration-after-statement -D__XEN_TOOLS__ -D_LARGEFILE_SOURCE
> -D_LARGEFILE64_SOURCE -m64 -fno-strict-aliasing -std=gnu99 -Wall
> -Wstrict-prototypes -Wno-unused-value -Wdeclaration-after-statement
> -D__XEN_TOOLS__ -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -m64
> -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes
> -Wno-unused-value -Wdeclaration-after-statement -D__XEN_TOOLS__
> -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -Werror -Wno-unused
> -I../../../tools/libxc -I../../../tools/include -I ../../libaio/src -I..
> -I../lib -I../../../tools/include -I. -I../../xenstore -D_GNU_SOURCE
> -Wp,-MD,.block-qcow2.o.d -c -o block-qcow2.o block-qcow2.c
> cc1: warnings being treated as errors
> block-qcow2.c: In function Œqcow2_create¹:
> block-qcow2.c:1312: warning: ignoring return value of Œwrite¹, declared
> with attribute warn_unused_result
> block-qcow2.c:1314: warning: ignoring return value of Œwrite¹, declared
> with attribute warn_unused_result
> block-qcow2.c:1319: warning: ignoring return value of Œwrite¹, declared
> with attribute warn_unused_result
> block-qcow2.c:1322: warning: ignoring return value of Œwrite¹, declared
> with attribute warn_unused_result
> block-qcow2.c:1325: warning: ignoring return value of Œwrite¹, declared
> with attribute warn_unused_result
> block-qcow2.c: In function Œqcow_write_compressed¹:
> block-qcow2.c:1354: warning: ignoring return value of Œftruncate¹,
> declared with attribute warn_unused_result
> .....
> .....
> 
> 
> Thanks,
> Maturi
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: xen-unstable build failure in tools/blktap/drivers for block-qcow2.c

[Kevin Wolf]

> Properly speaking we should check those write() return values anyway. If we
> don't want to deviate from the qemu original sources that much then we could
> special-case CFLAGS to remove -Wno-unused for just that file.

Shouldn't -Wno-unused disable the check rather than enabling it?
Removing it causes even more warnings. Maturi's compiler just seems to
ignore that switch.

I could compile the code with both gcc 4.1.2 and 4.3.0 without any such
problems.

Kevin

[PATCH] blktap: Fix unused warnings in block-qcow2.c

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

Kevin Wolf schrieb:
> Shouldn't -Wno-unused disable the check rather than enabling it?
> Removing it causes even more warnings. Maturi's compiler just seems to
> ignore that switch.

I see, it's the -D_FORTIFY_SOURCE=2 which causes the warnings. This is
not enabled in the standard xen-unstable Makefiles, so we could not
reproduce it.

The fix is trivial: These warning are all in functions which are used
only by qemu, but not by blktap. Just drop them.

Kevin

Signed-off-by: Kevin Wolf <kwolf@suse.de>

[-- Attachment #2: blktap-drop-unused.patch --]
[-- Type: text/x-patch, Size: 5253 bytes --]

diff -r 863563b3e029 tools/blktap/drivers/block-qcow2.c
--- a/tools/blktap/drivers/block-qcow2.c	Thu Feb 28 13:26:05 2008 +0100
+++ b/tools/blktap/drivers/block-qcow2.c	Thu Feb 28 13:30:19 2008 +0100
@@ -1241,167 +1241,6 @@ static void create_refcount_update(QCowC
 		refcount++;
 		*p = cpu_to_be16(refcount);
 	}
-}
-
-static int qcow2_create(const char *filename, int64_t total_size,
-		const char *backing_file, int flags)
-{
-	int fd, header_size, backing_filename_len, l1_size, i, shift, l2_bits;
-	QCowHeader header;
-	uint64_t tmp, offset;
-	QCowCreateState s1, *s = &s1;
-
-	memset(s, 0, sizeof(*s));
-
-	fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
-	if (fd < 0)
-		return -1;
-	memset(&header, 0, sizeof(header));
-	header.magic = cpu_to_be32(QCOW_MAGIC);
-	header.version = cpu_to_be32(QCOW_VERSION);
-	header.size = cpu_to_be64(total_size * 512);
-	header_size = sizeof(header);
-	backing_filename_len = 0;
-	if (backing_file) {
-		header.backing_file_offset = cpu_to_be64(header_size);
-		backing_filename_len = strlen(backing_file);
-		header.backing_file_size = cpu_to_be32(backing_filename_len);
-		header_size += backing_filename_len;
-	}
-	s->cluster_bits = 12;  /* 4 KB clusters */
-	s->cluster_size = 1 << s->cluster_bits;
-	header.cluster_bits = cpu_to_be32(s->cluster_bits);
-	header_size = (header_size + 7) & ~7;
-	if (flags & BLOCK_FLAG_ENCRYPT) {
-		header.crypt_method = cpu_to_be32(QCOW_CRYPT_AES);
-	} else {
-		header.crypt_method = cpu_to_be32(QCOW_CRYPT_NONE);
-	}
-	l2_bits = s->cluster_bits - 3;
-	shift = s->cluster_bits + l2_bits;
-	l1_size = (((total_size * 512) + (1LL << shift) - 1) >> shift);
-	offset = align_offset(header_size, s->cluster_size);
-	s->l1_table_offset = offset;
-	header.l1_table_offset = cpu_to_be64(s->l1_table_offset);
-	header.l1_size = cpu_to_be32(l1_size);
-	offset += align_offset(l1_size * sizeof(uint64_t), s->cluster_size);
-
-	s->refcount_table = qemu_mallocz(s->cluster_size);
-	if (!s->refcount_table)
-		goto fail;
-	s->refcount_block = qemu_mallocz(s->cluster_size);
-	if (!s->refcount_block)
-		goto fail;
-
-	s->refcount_table_offset = offset;
-	header.refcount_table_offset = cpu_to_be64(offset);
-	header.refcount_table_clusters = cpu_to_be32(1);
-	offset += s->cluster_size;
-
-	s->refcount_table[0] = cpu_to_be64(offset);
-	s->refcount_block_offset = offset;
-	offset += s->cluster_size;
-
-	/* update refcounts */
-	create_refcount_update(s, 0, header_size);
-	create_refcount_update(s, s->l1_table_offset, l1_size * sizeof(uint64_t));
-	create_refcount_update(s, s->refcount_table_offset, s->cluster_size);
-	create_refcount_update(s, s->refcount_block_offset, s->cluster_size);
-
-	/* write all the data */
-	write(fd, &header, sizeof(header));
-	if (backing_file) {
-		write(fd, backing_file, backing_filename_len);
-	}
-	lseek(fd, s->l1_table_offset, SEEK_SET);
-	tmp = 0;
-	for(i = 0;i < l1_size; i++) {
-		write(fd, &tmp, sizeof(tmp));
-	}
-	lseek(fd, s->refcount_table_offset, SEEK_SET);
-	write(fd, s->refcount_table, s->cluster_size);
-
-	lseek(fd, s->refcount_block_offset, SEEK_SET);
-	write(fd, s->refcount_block, s->cluster_size);
-
-	qemu_free(s->refcount_table);
-	qemu_free(s->refcount_block);
-	close(fd);
-	return 0;
-fail:
-	qemu_free(s->refcount_table);
-	qemu_free(s->refcount_block);
-	close(fd);
-	return -ENOMEM;
-}
-
-/* XXX: put compressed sectors first, then all the cluster aligned
-   tables to avoid losing bytes in alignment */
-static int qcow_write_compressed(struct disk_driver *bs, int64_t sector_num,
-		const uint8_t *buf, int nb_sectors)
-{
-	BDRVQcowState *s = bs->private;
-	z_stream strm;
-	int ret, out_len;
-	uint8_t *out_buf;
-	uint64_t cluster_offset;
-
-	if (nb_sectors == 0) {
-		/* align end of file to a sector boundary to ease reading with
-		   sector based I/Os */
-		cluster_offset = 512 * s->total_sectors;
-		cluster_offset = (cluster_offset + 511) & ~511;
-		ftruncate(s->fd, cluster_offset);
-		return 0;
-	}
-
-	if (nb_sectors != s->cluster_sectors)
-		return -EINVAL;
-
-	out_buf = qemu_malloc(s->cluster_size + (s->cluster_size / 1000) + 128);
-	if (!out_buf)
-		return -ENOMEM;
-
-	/* best compression, small window, no zlib header */
-	memset(&strm, 0, sizeof(strm));
-	ret = deflateInit2(&strm, Z_DEFAULT_COMPRESSION,
-			Z_DEFLATED, -12,
-			9, Z_DEFAULT_STRATEGY);
-	if (ret != 0) {
-		qemu_free(out_buf);
-		return -1;
-	}
-
-	strm.avail_in = s->cluster_size;
-	strm.next_in = (uint8_t *)buf;
-	strm.avail_out = s->cluster_size;
-	strm.next_out = out_buf;
-
-	ret = deflate(&strm, Z_FINISH);
-	if (ret != Z_STREAM_END && ret != Z_OK) {
-		qemu_free(out_buf);
-		deflateEnd(&strm);
-		return -1;
-	}
-	out_len = strm.next_out - out_buf;
-
-	deflateEnd(&strm);
-
-	if (ret != Z_STREAM_END || out_len >= s->cluster_size) {
-		/* could not compress: write normal cluster */
-		qcow_write(bs, sector_num, buf, s->cluster_sectors);
-	} else {
-		cluster_offset = get_cluster_offset(bs, sector_num << 9, 2,
-											out_len, 0, 0);
-		cluster_offset &= s->cluster_offset_mask;
-		if (bdrv_pwrite(s->fd, cluster_offset, out_buf, out_len) != out_len) {
-			qemu_free(out_buf);
-			return -1;
-		}
-	}
-
-	qemu_free(out_buf);
-	return 0;
 }
 
 static int qcow_submit(struct disk_driver *bs)

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: xen-unstable build failure in tools/blktap/drivers for block-qcow2.c

[Keir Fraser]

On 28/2/08 12:14, "Kevin Wolf" <kwolf@suse.de> wrote:

>> Properly speaking we should check those write() return values anyway. If we
>> don't want to deviate from the qemu original sources that much then we could
>> special-case CFLAGS to remove -Wno-unused for just that file.
> 
> Shouldn't -Wno-unused disable the check rather than enabling it?
> Removing it causes even more warnings. Maturi's compiler just seems to
> ignore that switch.
> 
> I could compile the code with both gcc 4.1.2 and 4.3.0 without any such
> problems.

Oh yes, silly me.

 -- Keir

Re: xen-unstable build failure in tools/blktap/drivers for block-qcow2.c

[Srinivas Maturi]

Keir Fraser wrote:
> Which version of gcc are you using? It works for me with 3.4.4 and 4.1.1.
>   

We are using gcc 4.1.2.

Thanks,
Maturi

Re: xen-unstable build failure in tools/blktap/drivers for block-qcow2.c

[Kevin Wolf]

Srinivas Maturi schrieb:
> We are using gcc 4.1.2.

Ok. As I wrote the problem is not with the compiler version but with
your compiler switches, namely -D_FORTIFY_SOURCE=2. Keir has commited my
patch already, so please try the latest xen-unstable.

Kevin

Re: xen-unstable build failure in tools/blktap/drivers for block-qcow2.c

[Srinivas Maturi]

Kevin Wolf wrote:
> Srinivas Maturi schrieb:
>   
>> We are using gcc 4.1.2.
>>     
>
> Ok. As I wrote the problem is not with the compiler version but with
> your compiler switches, namely -D_FORTIFY_SOURCE=2. Keir has commited my
> patch already, so please try the latest xen-unstable.
>
>
>   

The build succeeded with changeset 17152. Thanks for the patch.

Thanks,
Maturi