* [Qemu-devel] [PATCH 00/10] FORTIFY_SOURCE followup
@ 2010-03-03 12:06 Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 01/10] cow: return errno instead of -1 Juan Quintela
` (9 more replies)
0 siblings, 10 replies; 16+ messages in thread
From: Juan Quintela @ 2010-03-03 12:06 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, amit.shah
Hi
This series make:
- all block *_create() functions return -errno instead of -1
- this makes that we can end writting errno/error at bdrv_create()
callers (qemu-img)
- once there found a double free problem in the error handling of
vmdk, fixed it.
- slirp: also check that system() was able to fork (amit noticed it)
- daemonize: if we are unable to write into the pipe, print a message and exit.
We can't really recover from that error (amit noticed it).
Please review and apply.
Later, Juan.
Juan Quintela (10):
cow: return errno instead of -1
slirp: check system() success
qcow2: return errno instead of -1
qcow: return errno instead of -1
vmdk: return errno instead of -1
vl: exit if we are not able to write into the pipe
vmdk: make vmdk_snapshot_create return -errno
vmdk: fix double free
vmdk: share cleanup code
block: print errno on error
block/cow.c | 5 +--
block/qcow.c | 8 ++--
block/qcow2.c | 18 +++++-----
block/vmdk.c | 106 +++++++++++++++++++++++++++++++++++++--------------------
net/slirp.c | 2 +-
qemu-img.c | 4 +-
vl.c | 1 +
7 files changed, 88 insertions(+), 56 deletions(-)
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 01/10] cow: return errno instead of -1
2010-03-03 12:06 [Qemu-devel] [PATCH 00/10] FORTIFY_SOURCE followup Juan Quintela
@ 2010-03-03 12:06 ` Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 02/10] slirp: check system() success Juan Quintela
` (8 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Juan Quintela @ 2010-03-03 12:06 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, amit.shah
Remove not needed ret = 0 assignment.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
block/cow.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/block/cow.c b/block/cow.c
index 3733385..97e9745 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -224,7 +224,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
cow_fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
0644);
if (cow_fd < 0)
- return -1;
+ return -errno;
memset(&cow_header, 0, sizeof(cow_header));
cow_header.magic = cpu_to_be32(COW_MAGIC);
cow_header.version = cpu_to_be32(COW_VERSION);
@@ -251,7 +251,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
cow_header.size = cpu_to_be64(image_sectors * 512);
ret = qemu_write_full(cow_fd, &cow_header, sizeof(cow_header));
if (ret != sizeof(cow_header)) {
- ret = -1;
+ ret = -errno;
goto exit;
}
@@ -262,7 +262,6 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
goto exit;
}
- ret = 0;
exit:
close(cow_fd);
return ret;
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 02/10] slirp: check system() success
2010-03-03 12:06 [Qemu-devel] [PATCH 00/10] FORTIFY_SOURCE followup Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 01/10] cow: return errno instead of -1 Juan Quintela
@ 2010-03-03 12:06 ` Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 03/10] qcow2: return errno instead of -1 Juan Quintela
` (7 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Juan Quintela @ 2010-03-03 12:06 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, amit.shah
we shouldn't call W*() macros until we check that fork worked.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
net/slirp.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index 317cca7..7f846ec 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -472,7 +472,7 @@ static void slirp_smb_cleanup(SlirpState *s)
if (s->smb_dir[0] != '\0') {
snprintf(cmd, sizeof(cmd), "rm -rf %s", s->smb_dir);
ret = system(cmd);
- if (!WIFEXITED(ret)) {
+ if (ret == -1 || !WIFEXITED(ret)) {
qemu_error("'%s' failed.\n", cmd);
} else if (WEXITSTATUS(ret)) {
qemu_error("'%s' failed. Error code: %d\n",
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 03/10] qcow2: return errno instead of -1
2010-03-03 12:06 [Qemu-devel] [PATCH 00/10] FORTIFY_SOURCE followup Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 01/10] cow: return errno instead of -1 Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 02/10] slirp: check system() success Juan Quintela
@ 2010-03-03 12:06 ` Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 04/10] qcow: " Juan Quintela
` (6 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Juan Quintela @ 2010-03-03 12:06 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, amit.shah
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
block/qcow2.c | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index bf8170e..5b6dad9 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -851,7 +851,7 @@ static int qcow_create2(const char *filename, int64_t total_size,
fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
if (fd < 0)
- return -1;
+ return -errno;
memset(&header, 0, sizeof(header));
header.magic = cpu_to_be32(QCOW_MAGIC);
header.version = cpu_to_be32(QCOW_VERSION);
@@ -930,7 +930,7 @@ static int qcow_create2(const char *filename, int64_t total_size,
/* write all the data */
ret = qemu_write_full(fd, &header, sizeof(header));
if (ret != sizeof(header)) {
- ret = -1;
+ ret = -errno;
goto exit;
}
if (backing_file) {
@@ -943,25 +943,25 @@ static int qcow_create2(const char *filename, int64_t total_size,
cpu_to_be32s(&ext_bf.len);
ret = qemu_write_full(fd, &ext_bf, sizeof(ext_bf));
if (ret != sizeof(ext_bf)) {
- ret = -1;
+ ret = -errno;
goto exit;
}
ret = qemu_write_full(fd, backing_format, backing_format_len);
if (ret != backing_format_len) {
- ret = -1;
+ ret = -errno;
goto exit;
}
if (padding > 0) {
ret = qemu_write_full(fd, zero, padding);
if (ret != padding) {
- ret = -1;
+ ret = -errno;
goto exit;
}
}
}
ret = qemu_write_full(fd, backing_file, backing_filename_len);
if (ret != backing_filename_len) {
- ret = -1;
+ ret = -errno;
goto exit;
}
}
@@ -970,14 +970,14 @@ static int qcow_create2(const char *filename, int64_t total_size,
for(i = 0;i < l1_size; i++) {
ret = qemu_write_full(fd, &tmp, sizeof(tmp));
if (ret != sizeof(tmp)) {
- ret = -1;
+ ret = -errno;
goto exit;
}
}
lseek(fd, s->refcount_table_offset, SEEK_SET);
ret = qemu_write_full(fd, s->refcount_table, s->cluster_size);
if (ret != s->cluster_size) {
- ret = -1;
+ ret = -errno;
goto exit;
}
@@ -985,7 +985,7 @@ static int qcow_create2(const char *filename, int64_t total_size,
ret = qemu_write_full(fd, s->refcount_block,
ref_clusters * s->cluster_size);
if (ret != ref_clusters * s->cluster_size) {
- ret = -1;
+ ret = -errno;
goto exit;
}
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 04/10] qcow: return errno instead of -1
2010-03-03 12:06 [Qemu-devel] [PATCH 00/10] FORTIFY_SOURCE followup Juan Quintela
` (2 preceding siblings ...)
2010-03-03 12:06 ` [Qemu-devel] [PATCH 03/10] qcow2: return errno instead of -1 Juan Quintela
@ 2010-03-03 12:06 ` Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 05/10] vmdk: " Juan Quintela
` (5 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Juan Quintela @ 2010-03-03 12:06 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, amit.shah
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
block/qcow.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/block/qcow.c b/block/qcow.c
index 003db1e..c619984 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -766,7 +766,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options)
fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
if (fd < 0)
- return -1;
+ return -errno;
memset(&header, 0, sizeof(header));
header.magic = cpu_to_be32(QCOW_MAGIC);
header.version = cpu_to_be32(QCOW_VERSION);
@@ -804,14 +804,14 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options)
/* write all the data */
ret = qemu_write_full(fd, &header, sizeof(header));
if (ret != sizeof(header)) {
- ret = -1;
+ ret = -errno;
goto exit;
}
if (backing_file) {
ret = qemu_write_full(fd, backing_file, backing_filename_len);
if (ret != backing_filename_len) {
- ret = -1;
+ ret = -errno;
goto exit;
}
@@ -821,7 +821,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options)
for(i = 0;i < l1_size; i++) {
ret = qemu_write_full(fd, &tmp, sizeof(tmp));
if (ret != sizeof(tmp)) {
- ret = -1;
+ ret = -errno;
goto exit;
}
}
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 05/10] vmdk: return errno instead of -1
2010-03-03 12:06 [Qemu-devel] [PATCH 00/10] FORTIFY_SOURCE followup Juan Quintela
` (3 preceding siblings ...)
2010-03-03 12:06 ` [Qemu-devel] [PATCH 04/10] qcow: " Juan Quintela
@ 2010-03-03 12:06 ` Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 06/10] vl: exit if we are not able to write into the pipe Juan Quintela
` (4 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Juan Quintela @ 2010-03-03 12:06 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, amit.shah
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
block/vmdk.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 56c28a0..5b1d197 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -740,7 +740,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
0644);
if (fd < 0)
- return -1;
+ return -errno;
magic = cpu_to_be32(VMDK4_MAGIC);
memset(&header, 0, sizeof(header));
header.version = cpu_to_le32(1);
@@ -777,18 +777,18 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
/* write all the data */
ret = qemu_write_full(fd, &magic, sizeof(magic));
if (ret != sizeof(magic)) {
- ret = -1;
+ ret = -errno;
goto exit;
}
ret = qemu_write_full(fd, &header, sizeof(header));
if (ret != sizeof(header)) {
- ret = -1;
+ ret = -errno;
goto exit;
}
ret = ftruncate(fd, header.grain_offset << 9);
if (ret < 0) {
- ret = -1;
+ ret = -errno;
goto exit;
}
@@ -798,7 +798,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
i < gt_count; i++, tmp += gt_size) {
ret = qemu_write_full(fd, &tmp, sizeof(tmp));
if (ret != sizeof(tmp)) {
- ret = -1;
+ ret = -errno;
goto exit;
}
}
@@ -809,7 +809,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
i < gt_count; i++, tmp += gt_size) {
ret = qemu_write_full(fd, &tmp, sizeof(tmp));
if (ret != sizeof(tmp)) {
- ret = -1;
+ ret = -errno;
goto exit;
}
}
@@ -831,7 +831,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
lseek(fd, le64_to_cpu(header.desc_offset) << 9, SEEK_SET);
ret = qemu_write_full(fd, desc, strlen(desc));
if (ret != strlen(desc)) {
- ret = -1;
+ ret = -errno;
goto exit;
}
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 06/10] vl: exit if we are not able to write into the pipe
2010-03-03 12:06 [Qemu-devel] [PATCH 00/10] FORTIFY_SOURCE followup Juan Quintela
` (4 preceding siblings ...)
2010-03-03 12:06 ` [Qemu-devel] [PATCH 05/10] vmdk: " Juan Quintela
@ 2010-03-03 12:06 ` Juan Quintela
2010-03-03 13:51 ` [Qemu-devel] " Kevin Wolf
2010-03-03 12:06 ` [Qemu-devel] [PATCH 07/10] vmdk: make vmdk_snapshot_create return -errno Juan Quintela
` (3 subsequent siblings)
9 siblings, 1 reply; 16+ messages in thread
From: Juan Quintela @ 2010-03-03 12:06 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, amit.shah
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
vl.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/vl.c b/vl.c
index db7a178..119c7e4 100644
--- a/vl.c
+++ b/vl.c
@@ -5745,6 +5745,7 @@ int main(int argc, char **argv, char **envp)
uint8_t status = 1;
if (write(fds[1], &status, 1) != 1) {
perror("daemonize. Writing to pipe\n");
+ exit(1);
}
} else
#endif
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 07/10] vmdk: make vmdk_snapshot_create return -errno
2010-03-03 12:06 [Qemu-devel] [PATCH 00/10] FORTIFY_SOURCE followup Juan Quintela
` (5 preceding siblings ...)
2010-03-03 12:06 ` [Qemu-devel] [PATCH 06/10] vl: exit if we are not able to write into the pipe Juan Quintela
@ 2010-03-03 12:06 ` Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 08/10] vmdk: fix double free Juan Quintela
` (2 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Juan Quintela @ 2010-03-03 12:06 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, amit.shah
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
block/vmdk.c | 79 ++++++++++++++++++++++++++++++++++++++++++---------------
1 files changed, 58 insertions(+), 21 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 5b1d197..67a690e 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -187,6 +187,7 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
static int vmdk_snapshot_create(const char *filename, const char *backing_file)
{
int snp_fd, p_fd;
+ int ret;
uint32_t p_cid;
char *p_name, *gd_buf, *rgd_buf;
const char *real_filename, *temp_str;
@@ -211,35 +212,49 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
snp_fd = open(filename, O_RDWR | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, 0644);
if (snp_fd < 0)
- return -1;
+ return -errno;
p_fd = open(backing_file, O_RDONLY | O_BINARY | O_LARGEFILE);
if (p_fd < 0) {
close(snp_fd);
- return -1;
+ return -errno;
}
/* read the header */
- if (lseek(p_fd, 0x0, SEEK_SET) == -1)
+ if (lseek(p_fd, 0x0, SEEK_SET) == -1) {
+ ret = -errno;
goto fail;
- if (read(p_fd, hdr, HEADER_SIZE) != HEADER_SIZE)
+ }
+ if (read(p_fd, hdr, HEADER_SIZE) != HEADER_SIZE) {
+ ret = -errno;
goto fail;
+ }
/* write the header */
- if (lseek(snp_fd, 0x0, SEEK_SET) == -1)
+ if (lseek(snp_fd, 0x0, SEEK_SET) == -1) {
+ ret = -errno;
goto fail;
- if (write(snp_fd, hdr, HEADER_SIZE) == -1)
+ }
+ if (write(snp_fd, hdr, HEADER_SIZE) == -1) {
+ ret = -errno;
goto fail;
+ }
memset(&header, 0, sizeof(header));
memcpy(&header,&hdr[4], sizeof(header)); // skip the VMDK4_MAGIC
- if (ftruncate(snp_fd, header.grain_offset << 9))
+ if (ftruncate(snp_fd, header.grain_offset << 9)) {
+ ret = -errno;
goto fail;
+ }
/* the descriptor offset = 0x200 */
- if (lseek(p_fd, 0x200, SEEK_SET) == -1)
+ if (lseek(p_fd, 0x200, SEEK_SET) == -1) {
+ ret = -errno;
goto fail;
- if (read(p_fd, p_desc, DESC_SIZE) != DESC_SIZE)
+ }
+ if (read(p_fd, p_desc, DESC_SIZE) != DESC_SIZE) {
+ ret = -errno;
goto fail;
+ }
if ((p_name = strstr(p_desc,"CID")) != NULL) {
p_name += sizeof("CID");
@@ -258,10 +273,14 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
(uint32_t)header.capacity, real_filename);
/* write the descriptor */
- if (lseek(snp_fd, 0x200, SEEK_SET) == -1)
+ if (lseek(snp_fd, 0x200, SEEK_SET) == -1) {
+ ret = -errno;
goto fail;
- if (write(snp_fd, s_desc, strlen(s_desc)) == -1)
+ }
+ if (write(snp_fd, s_desc, strlen(s_desc)) == -1) {
+ ret = -errno;
goto fail;
+ }
gd_offset = header.gd_offset * SECTOR_SIZE; // offset of GD table
rgd_offset = header.rgd_offset * SECTOR_SIZE; // offset of RGD table
@@ -271,33 +290,51 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
* 512 GTE per GT, each GTE points to grain
*/
gt_size = (int64_t)header.num_gtes_per_gte * header.granularity * SECTOR_SIZE;
- if (!gt_size)
+ if (!gt_size) {
+ ret = -EINVAL;
goto fail;
+ }
gde_entries = (uint32_t)(capacity / gt_size); // number of gde/rgde
gd_size = gde_entries * sizeof(uint32_t);
/* write RGD */
rgd_buf = qemu_malloc(gd_size);
- if (lseek(p_fd, rgd_offset, SEEK_SET) == -1)
+ if (lseek(p_fd, rgd_offset, SEEK_SET) == -1) {
+ ret = -errno;
goto fail_rgd;
- if (read(p_fd, rgd_buf, gd_size) != gd_size)
+ }
+ if (read(p_fd, rgd_buf, gd_size) != gd_size) {
+ ret = -errno;
goto fail_rgd;
- if (lseek(snp_fd, rgd_offset, SEEK_SET) == -1)
+ }
+ if (lseek(snp_fd, rgd_offset, SEEK_SET) == -1) {
+ ret = -errno;
goto fail_rgd;
- if (write(snp_fd, rgd_buf, gd_size) == -1)
+ }
+ if (write(snp_fd, rgd_buf, gd_size) == -1) {
+ ret = -errno;
goto fail_rgd;
+ }
qemu_free(rgd_buf);
/* write GD */
gd_buf = qemu_malloc(gd_size);
- if (lseek(p_fd, gd_offset, SEEK_SET) == -1)
+ if (lseek(p_fd, gd_offset, SEEK_SET) == -1) {
+ ret = -errno;
goto fail_gd;
- if (read(p_fd, gd_buf, gd_size) != gd_size)
+ }
+ if (read(p_fd, gd_buf, gd_size) != gd_size) {
+ ret = -errno;
goto fail_gd;
- if (lseek(snp_fd, gd_offset, SEEK_SET) == -1)
+ }
+ if (lseek(snp_fd, gd_offset, SEEK_SET) == -1) {
+ ret = -errno;
goto fail_gd;
- if (write(snp_fd, gd_buf, gd_size) == -1)
+ }
+ if (write(snp_fd, gd_buf, gd_size) == -1) {
+ ret = -errno;
goto fail_gd;
+ }
qemu_free(gd_buf);
close(p_fd);
@@ -311,7 +348,7 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
fail:
close(p_fd);
close(snp_fd);
- return -1;
+ return ret;
}
static void vmdk_parent_close(BlockDriverState *bs)
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 08/10] vmdk: fix double free
2010-03-03 12:06 [Qemu-devel] [PATCH 00/10] FORTIFY_SOURCE followup Juan Quintela
` (6 preceding siblings ...)
2010-03-03 12:06 ` [Qemu-devel] [PATCH 07/10] vmdk: make vmdk_snapshot_create return -errno Juan Quintela
@ 2010-03-03 12:06 ` Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 09/10] vmdk: share cleanup code Juan Quintela
2010-03-03 12:07 ` [Qemu-devel] [PATCH 10/10] block: print errno on error Juan Quintela
9 siblings, 0 replies; 16+ messages in thread
From: Juan Quintela @ 2010-03-03 12:06 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, amit.shah
fail_gd error case would also free rgd_buf that was already freed
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
block/vmdk.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 67a690e..819c1c9 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -315,7 +315,6 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
ret = -errno;
goto fail_rgd;
}
- qemu_free(rgd_buf);
/* write GD */
gd_buf = qemu_malloc(gd_size);
@@ -336,6 +335,7 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
goto fail_gd;
}
qemu_free(gd_buf);
+ qemu_free(rgd_buf);
close(p_fd);
close(snp_fd);
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 09/10] vmdk: share cleanup code
2010-03-03 12:06 [Qemu-devel] [PATCH 00/10] FORTIFY_SOURCE followup Juan Quintela
` (7 preceding siblings ...)
2010-03-03 12:06 ` [Qemu-devel] [PATCH 08/10] vmdk: fix double free Juan Quintela
@ 2010-03-03 12:06 ` Juan Quintela
2010-03-03 12:07 ` [Qemu-devel] [PATCH 10/10] block: print errno on error Juan Quintela
9 siblings, 0 replies; 16+ messages in thread
From: Juan Quintela @ 2010-03-03 12:06 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, amit.shah
cleanup code is identical for error/success cases. Only difference
are goto labels.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
block/vmdk.c | 13 ++++---------
1 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 819c1c9..007fca4 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -334,18 +334,13 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
ret = -errno;
goto fail_gd;
}
- qemu_free(gd_buf);
- qemu_free(rgd_buf);
-
- close(p_fd);
- close(snp_fd);
- return 0;
+ ret = 0;
- fail_gd:
+fail_gd:
qemu_free(gd_buf);
- fail_rgd:
+fail_rgd:
qemu_free(rgd_buf);
- fail:
+fail:
close(p_fd);
close(snp_fd);
return ret;
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 10/10] block: print errno on error
2010-03-03 12:06 [Qemu-devel] [PATCH 00/10] FORTIFY_SOURCE followup Juan Quintela
` (8 preceding siblings ...)
2010-03-03 12:06 ` [Qemu-devel] [PATCH 09/10] vmdk: share cleanup code Juan Quintela
@ 2010-03-03 12:07 ` Juan Quintela
2010-03-03 14:01 ` [Qemu-devel] " Kevin Wolf
9 siblings, 1 reply; 16+ messages in thread
From: Juan Quintela @ 2010-03-03 12:07 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, amit.shah
Now that we changed all create calls to return errno, just print it.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
qemu-img.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 0c9f2d4..f6c40fb 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -374,7 +374,7 @@ static int img_create(int argc, char **argv)
} else if (ret == -EFBIG) {
error("The image size is too large for file format '%s'", fmt);
} else {
- error("Error while formatting");
+ error("Error (%s) while formatting for file format '%s'", strerror(ret), fmt);
}
}
return 0;
@@ -687,7 +687,7 @@ static int img_convert(int argc, char **argv)
} else if (ret == -EFBIG) {
error("The image size is too large for file format '%s'", out_fmt);
} else {
- error("Error while formatting '%s'", out_filename);
+ error("Error (%s) while formatting file '%s'", strerror(ret), out_filename);
}
}
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] Re: [PATCH 06/10] vl: exit if we are not able to write into the pipe
2010-03-03 12:06 ` [Qemu-devel] [PATCH 06/10] vl: exit if we are not able to write into the pipe Juan Quintela
@ 2010-03-03 13:51 ` Kevin Wolf
2010-03-03 14:00 ` Juan Quintela
0 siblings, 1 reply; 16+ messages in thread
From: Kevin Wolf @ 2010-03-03 13:51 UTC (permalink / raw)
To: Juan Quintela; +Cc: amit.shah, qemu-devel
Am 03.03.2010 13:06, schrieb Juan Quintela:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> vl.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index db7a178..119c7e4 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -5745,6 +5745,7 @@ int main(int argc, char **argv, char **envp)
> uint8_t status = 1;
> if (write(fds[1], &status, 1) != 1) {
> perror("daemonize. Writing to pipe\n");
> + exit(1);
> }
> } else
> #endif
This is already the handling code for a fatal error, it's just trying to
tell the parent process that it went wrong. Completing the context:
fprintf(stderr, "Could not acquire pid file: %s\n",
strerror(errno));
exit(1);
}
So there is already an exit(1), this patch doesn't change anything.
Kevin
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] Re: [PATCH 06/10] vl: exit if we are not able to write into the pipe
2010-03-03 13:51 ` [Qemu-devel] " Kevin Wolf
@ 2010-03-03 14:00 ` Juan Quintela
0 siblings, 0 replies; 16+ messages in thread
From: Juan Quintela @ 2010-03-03 14:00 UTC (permalink / raw)
To: Kevin Wolf; +Cc: amit.shah, qemu-devel
Kevin Wolf <kwolf@redhat.com> wrote:
> Am 03.03.2010 13:06, schrieb Juan Quintela:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>> vl.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index db7a178..119c7e4 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -5745,6 +5745,7 @@ int main(int argc, char **argv, char **envp)
>> uint8_t status = 1;
>> if (write(fds[1], &status, 1) != 1) {
>> perror("daemonize. Writing to pipe\n");
>> + exit(1);
>> }
>> } else
>> #endif
>
> This is already the handling code for a fatal error, it's just trying to
> tell the parent process that it went wrong. Completing the context:
>
> fprintf(stderr, "Could not acquire pid file: %s\n",
> strerror(errno));
> exit(1);
> }
>
> So there is already an exit(1), this patch doesn't change anything.
>
> Kevin
Else without braces :(
You are right. Would remove on respin.
Later, Juan.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] Re: [PATCH 10/10] block: print errno on error
2010-03-03 12:07 ` [Qemu-devel] [PATCH 10/10] block: print errno on error Juan Quintela
@ 2010-03-03 14:01 ` Kevin Wolf
2010-03-03 14:36 ` Paolo Bonzini
0 siblings, 1 reply; 16+ messages in thread
From: Kevin Wolf @ 2010-03-03 14:01 UTC (permalink / raw)
To: Juan Quintela; +Cc: amit.shah, qemu-devel
Am 03.03.2010 13:07, schrieb Juan Quintela:
> Now that we changed all create calls to return errno, just print it.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> qemu-img.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 0c9f2d4..f6c40fb 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -374,7 +374,7 @@ static int img_create(int argc, char **argv)
> } else if (ret == -EFBIG) {
> error("The image size is too large for file format '%s'", fmt);
> } else {
> - error("Error while formatting");
> + error("Error (%s) while formatting for file format '%s'", strerror(ret), fmt);
> }
> }
> return 0;
> @@ -687,7 +687,7 @@ static int img_convert(int argc, char **argv)
> } else if (ret == -EFBIG) {
> error("The image size is too large for file format '%s'", out_fmt);
> } else {
> - error("Error while formatting '%s'", out_filename);
> + error("Error (%s) while formatting file '%s'", strerror(ret), out_filename);
> }
> }
>
I think it should be strerror(-ret) in both cases.
Kevin
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] Re: [PATCH 10/10] block: print errno on error
2010-03-03 14:01 ` [Qemu-devel] " Kevin Wolf
@ 2010-03-03 14:36 ` Paolo Bonzini
2010-03-03 16:21 ` Juan Quintela
0 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2010-03-03 14:36 UTC (permalink / raw)
To: Kevin Wolf; +Cc: amit.shah, qemu-devel, Juan Quintela
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 0c9f2d4..f6c40fb 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -374,7 +374,7 @@ static int img_create(int argc, char **argv)
>> } else if (ret == -EFBIG) {
>> error("The image size is too large for file format '%s'", fmt);
>> } else {
>> - error("Error while formatting");
>> + error("Error (%s) while formatting for file format '%s'", strerror(ret), fmt);
>> }
>> }
>> return 0;
>> @@ -687,7 +687,7 @@ static int img_convert(int argc, char **argv)
>> } else if (ret == -EFBIG) {
>> error("The image size is too large for file format '%s'", out_fmt);
>> } else {
>> - error("Error while formatting '%s'", out_filename);
>> + error("Error (%s) while formatting file '%s'", strerror(ret), out_filename);
>> }
>> }
>>
>
> I think it should be strerror(-ret) in both cases.
Yes; also, since you are at it, I think that respectively
error("%s: error while creating %s image: %s", filename, fmt,
strerror(-ret);
error(%s: error while converting to %s: %s", out_filename, fmt,
strerror(-ret);
would be more consistent with usual error messages:
$ cat fdsfds
cat: fdsfds: No such file or directory
Paolo
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] Re: [PATCH 10/10] block: print errno on error
2010-03-03 14:36 ` Paolo Bonzini
@ 2010-03-03 16:21 ` Juan Quintela
0 siblings, 0 replies; 16+ messages in thread
From: Juan Quintela @ 2010-03-03 16:21 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Kevin Wolf, amit.shah, qemu-devel
Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> diff --git a/qemu-img.c b/qemu-img.c
>>> index 0c9f2d4..f6c40fb 100644
>>> --- a/qemu-img.c
>>> +++ b/qemu-img.c
>>> @@ -374,7 +374,7 @@ static int img_create(int argc, char **argv)
>>> } else if (ret == -EFBIG) {
>>> error("The image size is too large for file format '%s'", fmt);
>>> } else {
>>> - error("Error while formatting");
>>> + error("Error (%s) while formatting for file format '%s'", strerror(ret), fmt);
>>> }
>>> }
>>> return 0;
>>> @@ -687,7 +687,7 @@ static int img_convert(int argc, char **argv)
>>> } else if (ret == -EFBIG) {
>>> error("The image size is too large for file format '%s'", out_fmt);
>>> } else {
>>> - error("Error while formatting '%s'", out_filename);
>>> + error("Error (%s) while formatting file '%s'", strerror(ret), out_filename);
>>> }
>>> }
>>>
>>
>> I think it should be strerror(-ret) in both cases.
oops, yes.
> Yes; also, since you are at it, I think that respectively
>
> error("%s: error while creating %s image: %s", filename, fmt,
> strerror(-ret);
>
> error(%s: error while converting to %s: %s", out_filename, fmt,
> strerror(-ret);
>
> would be more consistent with usual error messages:
>
> $ cat fdsfds
> cat: fdsfds: No such file or directory
I agree. I just didn't want to change it too much.
> Paolo
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2010-03-03 16:21 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-03 12:06 [Qemu-devel] [PATCH 00/10] FORTIFY_SOURCE followup Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 01/10] cow: return errno instead of -1 Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 02/10] slirp: check system() success Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 03/10] qcow2: return errno instead of -1 Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 04/10] qcow: " Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 05/10] vmdk: " Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 06/10] vl: exit if we are not able to write into the pipe Juan Quintela
2010-03-03 13:51 ` [Qemu-devel] " Kevin Wolf
2010-03-03 14:00 ` Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 07/10] vmdk: make vmdk_snapshot_create return -errno Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 08/10] vmdk: fix double free Juan Quintela
2010-03-03 12:06 ` [Qemu-devel] [PATCH 09/10] vmdk: share cleanup code Juan Quintela
2010-03-03 12:07 ` [Qemu-devel] [PATCH 10/10] block: print errno on error Juan Quintela
2010-03-03 14:01 ` [Qemu-devel] " Kevin Wolf
2010-03-03 14:36 ` Paolo Bonzini
2010-03-03 16:21 ` Juan Quintela
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.