[Qemu-devel] [PATCH 5/7] Consolidate printing of block driver options
From: Jes Sorensen
This consolidates the printing of block driver options in
print_block_option_help() which is called from both img_create() and
img_convert().
This allows for the "?" detection to be done just after the parsing of
options and the filename, instead of half way down the codepath of
these functions.
Signed-off-by: Jes Sorensen
---
qemu-img.c | 46 +-
1 files changed, 37 insertions(+), 9 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 7f4939e..c7d0ca8 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -188,6 +188,33 @@ static int read_password(char *buf, int buf_size)
}
#endif
+static int print_block_option_help(const char *filename, const char *fmt)
+{
+BlockDriver *drv, *proto_drv;
+QEMUOptionParameter *create_options = NULL;
+
+/* Find driver and parse its options */
+drv = bdrv_find_format(fmt);
+if (!drv) {
+error("Unknown file format '%s'", fmt);
+return 1;
+}
+
+proto_drv = bdrv_find_protocol(filename);
+if (!proto_drv) {
+error("Unknown protocol '%s'", filename);
+return 1;
+}
+
+create_options = append_option_parameters(create_options,
+ drv->create_options);
+create_options = append_option_parameters(create_options,
+ proto_drv->create_options);
+print_option_help(create_options);
+free_option_parameters(create_options);
+return 0;
+}
+
static BlockDriverState *bdrv_new_open(const char *filename,
const char *fmt,
int flags)
@@ -310,6 +337,11 @@ static int img_create(int argc, char **argv)
help();
filename = argv[optind++];
+if (options && !strcmp(options, "?")) {
+ret = print_block_option_help(filename, fmt);
+goto out;
+}
+
/* Find driver and parse its options */
drv = bdrv_find_format(fmt);
if (!drv) {
@@ -330,11 +362,6 @@ static int img_create(int argc, char **argv)
create_options = append_option_parameters(create_options,
proto_drv->create_options);
-if (options && !strcmp(options, "?")) {
-print_option_help(create_options);
-goto out;
-}
-
/* Create parameter list with default values */
param = parse_option_parameters("", create_options, param);
set_option_parameter_int(param, BLOCK_OPT_SIZE, -1);
@@ -696,6 +723,11 @@ static int img_convert(int argc, char **argv)
out_filename = argv[argc - 1];
+if (options && !strcmp(options, "?")) {
+ret = print_block_option_help(out_filename, out_fmt);
+goto out;
+}
+
if (bs_n > 1 && out_baseimg) {
error("-B makes no sense when concatenating multiple input images");
ret = -1;
@@ -748,10 +780,6 @@ static int img_convert(int argc, char **argv)
drv->create_options);
create_options = append_option_parameters(create_options,
proto_drv->create_options);
-if (options && !strcmp(options, "?")) {
-print_option_help(create_options);
-goto out;
-}
if (options) {
param = parse_option_parameters(options, create_options, param);
--
1.7.3.2
[Qemu-devel] [PATCH 2/7] Use qemu_mallocz() instead of calloc() in img_convert()
From: Jes Sorensen
Signed-off-by: Jes Sorensen
---
qemu-img.c |8 ++--
1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index fa77ac0..eca99c4 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -699,11 +699,7 @@ static int img_convert(int argc, char **argv)
return 1;
}
-bs = calloc(bs_n, sizeof(BlockDriverState *));
-if (!bs) {
-error("Out of memory");
-return 1;
-}
+bs = qemu_mallocz(bs_n * sizeof(BlockDriverState *));
total_sectors = 0;
for (bs_i = 0; bs_i < bs_n; bs_i++) {
@@ -983,7 +979,7 @@ out:
bdrv_delete(bs[bs_i]);
}
}
-free(bs);
+qemu_free(bs);
if (ret) {
return 1;
}
--
1.7.3.2
[Qemu-devel] [PATCH 1/7] Add missing tracing to qemu_mallocz()
From: Jes Sorensen
Signed-off-by: Jes Sorensen
---
qemu-malloc.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/qemu-malloc.c b/qemu-malloc.c
index 28fb05a..b9b3851 100644
--- a/qemu-malloc.c
+++ b/qemu-malloc.c
@@ -64,10 +64,13 @@ void *qemu_realloc(void *ptr, size_t size)
void *qemu_mallocz(size_t size)
{
+void *ptr;
if (!size && !allow_zero_malloc()) {
abort();
}
-return qemu_oom_check(calloc(1, size ? size : 1));
+ptr = qemu_oom_check(calloc(1, size ? size : 1));
+trace_qemu_malloc(size, ptr);
+return ptr;
}
char *qemu_strdup(const char *str)
--
1.7.3.2
[Qemu-devel] [PATCH v3 0/7] Cleanup qemu-img code
From: Jes Sorensen Hi, These patches applies a number of cleanups to qemu-img.c as well as a minor bug in qemu-malloc.c. The handling of block help printing is moved to shared code, which allows the "?" detection to happen early in the parsing, instead of half way down img_create() and img_convert(). I would like to see this happen as I would like to pull some of the code out of img_create() and into block.c so it can be shared with qemu and qemu-img. In addition there is a couple of patches to clean up the error handling in qemu-img.c and make it more consistent. The formatting patch is solely because the last patch wanted to change code next to the badly formatted code, and I didn't want to pollute the patch with the formatting fixed. The seventh patch fixes qemu-img to exit on detection of unknown options instead of continuing with a potentially wrong set of arguments. v3 applies a number of changes discussed on irc and email. This is the grow to seven from three patches series. Cheers, Jes Jes Sorensen (7): Add missing tracing to qemu_mallocz() Use qemu_mallocz() instead of calloc() in img_convert() img_convert(): Only try to free bs[] entries if bs is valid. Make error handling more consistent in img_create() and img_resize() Consolidate printing of block driver options Fix formatting and missing braces in qemu-img.c Fail if detecting an unknown option qemu-img.c| 162 +++- qemu-malloc.c |5 ++- 2 files changed, 117 insertions(+), 50 deletions(-) -- 1.7.3.2
Re: [Qemu-devel] [PATCH 1/3] Consolidate printing of block driver options
On 12/06/10 11:37, Stefan Hajnoczi wrote: > On Mon, Dec 6, 2010 at 10:20 AM, Jes Sorensen wrote: >> On 12/06/10 10:32, Stefan Hajnoczi wrote: >>> On Mon, Dec 6, 2010 at 8:17 AM, wrote: >>> Why goto out2 and not just return like the bs > 1 && out_baseimg check? >> >> It is cleaner, I'd rather convert the bs_n test to do it too. > > "out2" tells me nothing and is just indirection for a return. At this > point no resources have been acquired and it is simplest to bail out > early. A consistent out path is more likely to catch issues if the code is modified later. I am not a big fan of the random mix of return vs goto out that we spray over the code or having help() call exit() for that matter. Cheers, Jes
Re: [Qemu-devel] [PATCH 1/3] Consolidate printing of block driver options
On 12/06/10 10:32, Stefan Hajnoczi wrote:
> On Mon, Dec 6, 2010 at 8:17 AM, wrote:
>> @@ -694,6 +721,11 @@ static int img_convert(int argc, char **argv)
>>
>> out_filename = argv[argc - 1];
>>
>> +if (options && !strcmp(options, "?")) {
>> +ret = print_block_option_help(out_filename, out_fmt);
>> +goto out2;
>> +}
>> +
>> if (bs_n > 1 && out_baseimg) {
>> error("-B makes no sense when concatenating multiple input images");
>> return 1;
>
> Why goto out2 and not just return like the bs > 1 && out_baseimg check?
It is cleaner, I'd rather convert the bs_n test to do it too.
Cheers,
Jes
[Qemu-devel] Re: [PATCH 3/3] Fail if detecting an unknown option
On 12/06/10 10:37, Kevin Wolf wrote: > Am 06.12.2010 09:02, schrieb Jes Sorensen: >> On 12/03/10 13:30, Kevin Wolf wrote: >> There is a perfectly logical explanation for that. Doing that would >> require for me to have clue, which is a bit much to expect :) >> >> That said, we should really do the same for the c == -1 case as well. > > That's what I thought at first, too. But then the break relates to the > switch instead of the for, so it would have to become a goto to a new > label. Probably not a big improvement... Yeah, it hit me the moment I hit send, so ignore that comment. Cheers, Jes
[Qemu-devel] [PATCH 1/3] Consolidate printing of block driver options
From: Jes Sorensen
This consolidates the printing of block driver options in
print_block_option_help() which is called from both img_create() and
img_convert().
This allows for the "?" detection to be done just after the parsing of
options and the filename, instead of half way down the codepath of
these functions.
Signed-off-by: Jes Sorensen
---
qemu-img.c | 47 ++-
1 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index fa77ac0..7863835 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -188,6 +188,33 @@ static int read_password(char *buf, int buf_size)
}
#endif
+static int print_block_option_help(const char *filename, const char *fmt)
+{
+BlockDriver *drv, *proto_drv;
+QEMUOptionParameter *create_options = NULL;
+
+/* Find driver and parse its options */
+drv = bdrv_find_format(fmt);
+if (!drv) {
+error("Unknown file format '%s'", fmt);
+return 1;
+}
+
+proto_drv = bdrv_find_protocol(filename);
+if (!proto_drv) {
+error("Unknown protocol '%s'", filename);
+return 1;
+}
+
+create_options = append_option_parameters(create_options,
+ drv->create_options);
+create_options = append_option_parameters(create_options,
+ proto_drv->create_options);
+print_option_help(create_options);
+free_option_parameters(create_options);
+return 0;
+}
+
static BlockDriverState *bdrv_new_open(const char *filename,
const char *fmt,
int flags)
@@ -310,6 +337,11 @@ static int img_create(int argc, char **argv)
help();
filename = argv[optind++];
+if (options && !strcmp(options, "?")) {
+ret = print_block_option_help(filename, fmt);
+goto out;
+}
+
/* Find driver and parse its options */
drv = bdrv_find_format(fmt);
if (!drv) {
@@ -328,11 +360,6 @@ static int img_create(int argc, char **argv)
create_options = append_option_parameters(create_options,
proto_drv->create_options);
-if (options && !strcmp(options, "?")) {
-print_option_help(create_options);
-goto out;
-}
-
/* Create parameter list with default values */
param = parse_option_parameters("", create_options, param);
set_option_parameter_int(param, BLOCK_OPT_SIZE, -1);
@@ -694,6 +721,11 @@ static int img_convert(int argc, char **argv)
out_filename = argv[argc - 1];
+if (options && !strcmp(options, "?")) {
+ret = print_block_option_help(out_filename, out_fmt);
+goto out2;
+}
+
if (bs_n > 1 && out_baseimg) {
error("-B makes no sense when concatenating multiple input images");
return 1;
@@ -749,10 +781,6 @@ static int img_convert(int argc, char **argv)
drv->create_options);
create_options = append_option_parameters(create_options,
proto_drv->create_options);
-if (options && !strcmp(options, "?")) {
-print_option_help(create_options);
-goto out;
-}
if (options) {
param = parse_option_parameters(options, create_options, param);
@@ -984,6 +1012,7 @@ out:
}
}
free(bs);
+out2:
if (ret) {
return 1;
}
--
1.7.3.2
[Qemu-devel] [PATCH 3/3] Fail if detecting an unknown option
From: Jes Sorensen
This patch changes qemu-img to exit if an unknown option is detected,
instead of trying to continue with a set of arguments which may be
incorrect.
Signed-off-by: Jes Sorensen
---
qemu-img.c |8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 2a54ae2..3e3ca36 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -309,6 +309,7 @@ static int img_create(int argc, char **argv)
break;
}
switch(c) {
+case '?':
case 'h':
help();
break;
@@ -477,6 +478,7 @@ static int img_check(int argc, char **argv)
break;
}
switch(c) {
+case '?':
case 'h':
help();
break;
@@ -555,6 +557,7 @@ static int img_commit(int argc, char **argv)
break;
}
switch(c) {
+case '?':
case 'h':
help();
break;
@@ -693,6 +696,7 @@ static int img_convert(int argc, char **argv)
break;
}
switch(c) {
+case '?':
case 'h':
help();
break;
@@ -1099,6 +1103,7 @@ static int img_info(int argc, char **argv)
break;
}
switch(c) {
+case '?':
case 'h':
help();
break;
@@ -1176,6 +1181,7 @@ static int img_snapshot(int argc, char **argv)
break;
}
switch(c) {
+case '?':
case 'h':
help();
return 0;
@@ -1291,6 +1297,7 @@ static int img_rebase(int argc, char **argv)
break;
}
switch(c) {
+case '?':
case 'h':
help();
return 0;
@@ -1505,6 +1512,7 @@ static int img_resize(int argc, char **argv)
break;
}
switch(c) {
+case '?':
case 'h':
help();
break;
--
1.7.3.2
[Qemu-devel] [PATCH 2/3] Fix formatting and missing braces in qemu-img.c
From: Jes Sorensen
Signed-off-by: Jes Sorensen
Reviewed-by: Stefan Hajnoczi
---
qemu-img.c | 77 +++
1 files changed, 51 insertions(+), 26 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 7863835..2a54ae2 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -305,8 +305,9 @@ static int img_create(int argc, char **argv)
flags = 0;
for(;;) {
c = getopt(argc, argv, "F:b:f:he6o:");
-if (c == -1)
+if (c == -1) {
break;
+}
switch(c) {
case 'h':
help();
@@ -333,8 +334,9 @@ static int img_create(int argc, char **argv)
}
/* Get the filename */
-if (optind >= argc)
+if (optind >= argc) {
help();
+}
filename = argv[optind++];
if (options && !strcmp(options, "?")) {
@@ -471,8 +473,9 @@ static int img_check(int argc, char **argv)
fmt = NULL;
for(;;) {
c = getopt(argc, argv, "f:h");
-if (c == -1)
+if (c == -1) {
break;
+}
switch(c) {
case 'h':
help();
@@ -482,8 +485,9 @@ static int img_check(int argc, char **argv)
break;
}
}
-if (optind >= argc)
+if (optind >= argc) {
help();
+}
filename = argv[optind++];
bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS);
@@ -547,8 +551,9 @@ static int img_commit(int argc, char **argv)
fmt = NULL;
for(;;) {
c = getopt(argc, argv, "f:h");
-if (c == -1)
+if (c == -1) {
break;
+}
switch(c) {
case 'h':
help();
@@ -558,8 +563,9 @@ static int img_commit(int argc, char **argv)
break;
}
}
-if (optind >= argc)
+if (optind >= argc) {
help();
+}
filename = argv[optind++];
bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR);
@@ -683,8 +689,9 @@ static int img_convert(int argc, char **argv)
flags = 0;
for(;;) {
c = getopt(argc, argv, "f:O:B:s:hce6o:");
-if (c == -1)
+if (c == -1) {
break;
+}
switch(c) {
case 'h':
help();
@@ -717,7 +724,9 @@ static int img_convert(int argc, char **argv)
}
bs_n = argc - optind - 1;
-if (bs_n < 1) help();
+if (bs_n < 1) {
+help();
+}
out_filename = argv[argc - 1];
@@ -908,8 +917,9 @@ static int img_convert(int argc, char **argv)
}
assert (remainder == 0);
-if (n < cluster_sectors)
+if (n < cluster_sectors) {
memset(buf + n * 512, 0, cluster_size - n * 512);
+}
if (is_not_zero(buf, cluster_size)) {
ret = bdrv_write_compressed(out_bs, sector_num, buf,
cluster_sectors);
@@ -929,12 +939,14 @@ static int img_convert(int argc, char **argv)
sector_num = 0; // total number of sectors converted so far
for(;;) {
nb_sectors = total_sectors - sector_num;
-if (nb_sectors <= 0)
+if (nb_sectors <= 0) {
break;
-if (nb_sectors >= (IO_BUF_SIZE / 512))
+}
+if (nb_sectors >= (IO_BUF_SIZE / 512)) {
n = (IO_BUF_SIZE / 512);
-else
+} else {
n = nb_sectors;
+}
while (sector_num - bs_offset >= bs_sectors) {
bs_i ++;
@@ -946,8 +958,9 @@ static int img_convert(int argc, char **argv)
sector_num, bs_i, bs_offset, bs_sectors); */
}
-if (n > bs_offset + bs_sectors - sector_num)
+if (n > bs_offset + bs_sectors - sector_num) {
n = bs_offset + bs_sectors - sector_num;
+}
if (has_zero_init) {
/* If the output image is being created as a copy on write
image,
@@ -1082,8 +1095,9 @@ static int img_info(int argc, char **argv)
fmt = NULL;
for(;;) {
c = getopt(argc, argv, "f:h");
-if (c == -1)
+if (c == -1) {
break;
+}
switch(c) {
case 'h':
help();
@@ -1093,8 +1107,9 @@ static int img_info(int argc, char **argv)
break;
}
}
-if (optind >= argc)
+if (optind >= argc) {
help();
+}
filename = argv[optind++];
bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING);
@@ -1105,11 +1120,12 @@ static int img_info(int argc, char **argv)
bdrv_get_geometry(bs, &total_sectors);
get_human_readable_size(size_buf, sizeof(size_buf), total_sectors * 512);
allocated_size = g
[Qemu-devel] [PATCH v2 0/3] Cleanup qemu-img code
From: Jes Sorensen Hi, These patches moves the handling of block help printing to shared code, which allows the "?" detection to happen early in the parsing, instead of half way down img_create() and img_convert(). I would like to see this happen as I would like to pull some of the code out of img_create() and into block.c so it can be shared with qemu and qemu-img. The formatting patch is solely because the third patch wanted to change code next to the badly formatted code, and I didn't want to pollute the patch with the formatting fixed. The third patch fixes qemu-img to exit on detection of unknown options instead of continuing with a potentially wrong set of arguments. New in v2: Add missing free_option_parameters() and handle the help() case in the general switch() statements for the getopt() output. Cheers, Jes Jes Sorensen (3): Consolidate printing of block driver options Fix formatting and missing braces in qemu-img.c Fail if detecting an unknown option qemu-img.c | 132 1 files changed, 97 insertions(+), 35 deletions(-) -- 1.7.3.2
[Qemu-devel] Re: [PATCH 3/3] Fail if detecting an unknown option
On 12/03/10 13:30, Kevin Wolf wrote:
> Am 02.12.2010 18:46, schrieb jes.soren...@redhat.com:
>> diff --git a/qemu-img.c b/qemu-img.c
>> index d0dc445..f2e1c94 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -304,6 +304,12 @@ static int img_create(int argc, char **argv)
>> flags = 0;
>> for(;;) {
>> c = getopt(argc, argv, "F:b:f:he6o:");
>> +/*
>> + * Fail if we detect an unknown argument
>> + */
>> +if (c == '?') {
>> +return 1;
>> +}
>> if (c == -1) {
>> break;
>> }
>
> Why not making it another case in the switch statement below instead of
> an additional if?
There is a perfectly logical explanation for that. Doing that would
require for me to have clue, which is a bit much to expect :)
That said, we should really do the same for the c == -1 case as well.
Fixed in next version.
Cheers,
Jes
Re: [Qemu-devel] [PATCH 3/3] Fail if detecting an unknown option
On 12/03/10 12:46, Stefan Hajnoczi wrote: > On Thu, Dec 2, 2010 at 5:46 PM, wrote: >> From: Jes Sorensen >> >> This patch changes qemu-img to exit if an unknown option is detected, >> instead of trying to continue with a set of arguments which may be >> incorrect. >> >> Signed-off-by: Jes Sorensen >> --- >> qemu-img.c | 48 >> 1 files changed, 48 insertions(+), 0 deletions(-) > > Do we get a silent exit if an unknown option is detected? Normally > programs print their help/usage when this happens. > > Stefan Fixed in the next version :) Cheers, Jes
Re: [Qemu-devel] [PATCH 1/3] Consolidate printing of block driver options
On 12/03/10 11:57, Stefan Hajnoczi wrote:
> On Thu, Dec 2, 2010 at 5:46 PM, wrote:
>> +create_options = append_option_parameters(create_options,
>> + drv->create_options);
>> +create_options = append_option_parameters(create_options,
>> + proto_drv->create_options);
>> +print_option_help(create_options);
>
> free_option_parameters(create_options);
Hmmm good point
>> @@ -694,6 +720,11 @@ static int img_convert(int argc, char **argv)
>>
>> out_filename = argv[argc - 1];
>>
>> +if (options && !strcmp(options, "?")) {
>> +ret = print_block_option_help(out_filename, out_fmt);
>> +goto out2;
>> +}
>> +
>> if (bs_n > 1 && out_baseimg) {
>> error("-B makes no sense when concatenating multiple input images");
>> return 1;
>> @@ -749,10 +780,6 @@ static int img_convert(int argc, char **argv)
>> drv->create_options);
>> create_options = append_option_parameters(create_options,
>> proto_drv->create_options);
>> -if (options && !strcmp(options, "?")) {
>> -print_option_help(create_options);
>> -goto out;
>> -}
>>
>> if (options) {
>> param = parse_option_parameters(options, create_options, param);
>> @@ -984,6 +1011,7 @@ out:
>> }
>> }
>> free(bs);
>> +out2:
>
> Not needed, out is fine. All those free functions are nops on NULL pointers.
Actually tried that, but it segfaulted, which is why I added out2.
Cheers,
Jes
[Qemu-devel] [PATCH 2/3] Fix formatting and missing braces in qemu-img.c
From: Jes Sorensen
Signed-off-by: Jes Sorensen
---
qemu-img.c | 77 +++
1 files changed, 51 insertions(+), 26 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 99f30b3..d0dc445 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -304,8 +304,9 @@ static int img_create(int argc, char **argv)
flags = 0;
for(;;) {
c = getopt(argc, argv, "F:b:f:he6o:");
-if (c == -1)
+if (c == -1) {
break;
+}
switch(c) {
case 'h':
help();
@@ -332,8 +333,9 @@ static int img_create(int argc, char **argv)
}
/* Get the filename */
-if (optind >= argc)
+if (optind >= argc) {
help();
+}
filename = argv[optind++];
if (options && !strcmp(options, "?")) {
@@ -470,8 +472,9 @@ static int img_check(int argc, char **argv)
fmt = NULL;
for(;;) {
c = getopt(argc, argv, "f:h");
-if (c == -1)
+if (c == -1) {
break;
+}
switch(c) {
case 'h':
help();
@@ -481,8 +484,9 @@ static int img_check(int argc, char **argv)
break;
}
}
-if (optind >= argc)
+if (optind >= argc) {
help();
+}
filename = argv[optind++];
bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS);
@@ -546,8 +550,9 @@ static int img_commit(int argc, char **argv)
fmt = NULL;
for(;;) {
c = getopt(argc, argv, "f:h");
-if (c == -1)
+if (c == -1) {
break;
+}
switch(c) {
case 'h':
help();
@@ -557,8 +562,9 @@ static int img_commit(int argc, char **argv)
break;
}
}
-if (optind >= argc)
+if (optind >= argc) {
help();
+}
filename = argv[optind++];
bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR);
@@ -682,8 +688,9 @@ static int img_convert(int argc, char **argv)
flags = 0;
for(;;) {
c = getopt(argc, argv, "f:O:B:s:hce6o:");
-if (c == -1)
+if (c == -1) {
break;
+}
switch(c) {
case 'h':
help();
@@ -716,7 +723,9 @@ static int img_convert(int argc, char **argv)
}
bs_n = argc - optind - 1;
-if (bs_n < 1) help();
+if (bs_n < 1) {
+help();
+}
out_filename = argv[argc - 1];
@@ -907,8 +916,9 @@ static int img_convert(int argc, char **argv)
}
assert (remainder == 0);
-if (n < cluster_sectors)
+if (n < cluster_sectors) {
memset(buf + n * 512, 0, cluster_size - n * 512);
+}
if (is_not_zero(buf, cluster_size)) {
ret = bdrv_write_compressed(out_bs, sector_num, buf,
cluster_sectors);
@@ -928,12 +938,14 @@ static int img_convert(int argc, char **argv)
sector_num = 0; // total number of sectors converted so far
for(;;) {
nb_sectors = total_sectors - sector_num;
-if (nb_sectors <= 0)
+if (nb_sectors <= 0) {
break;
-if (nb_sectors >= (IO_BUF_SIZE / 512))
+}
+if (nb_sectors >= (IO_BUF_SIZE / 512)) {
n = (IO_BUF_SIZE / 512);
-else
+} else {
n = nb_sectors;
+}
while (sector_num - bs_offset >= bs_sectors) {
bs_i ++;
@@ -945,8 +957,9 @@ static int img_convert(int argc, char **argv)
sector_num, bs_i, bs_offset, bs_sectors); */
}
-if (n > bs_offset + bs_sectors - sector_num)
+if (n > bs_offset + bs_sectors - sector_num) {
n = bs_offset + bs_sectors - sector_num;
+}
if (has_zero_init) {
/* If the output image is being created as a copy on write
image,
@@ -1081,8 +1094,9 @@ static int img_info(int argc, char **argv)
fmt = NULL;
for(;;) {
c = getopt(argc, argv, "f:h");
-if (c == -1)
+if (c == -1) {
break;
+}
switch(c) {
case 'h':
help();
@@ -1092,8 +1106,9 @@ static int img_info(int argc, char **argv)
break;
}
}
-if (optind >= argc)
+if (optind >= argc) {
help();
+}
filename = argv[optind++];
bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING);
@@ -1104,11 +1119,12 @@ static int img_info(int argc, char **argv)
bdrv_get_geometry(bs, &total_sectors);
get_human_readable_size(size_buf, sizeof(size_buf), total_sectors * 512);
allocated_size = get_allocated_file_size(filename);
-if (al
[Qemu-devel] [PATCH 3/3] Fail if detecting an unknown option
From: Jes Sorensen
This patch changes qemu-img to exit if an unknown option is detected,
instead of trying to continue with a set of arguments which may be
incorrect.
Signed-off-by: Jes Sorensen
---
qemu-img.c | 48
1 files changed, 48 insertions(+), 0 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index d0dc445..f2e1c94 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -304,6 +304,12 @@ static int img_create(int argc, char **argv)
flags = 0;
for(;;) {
c = getopt(argc, argv, "F:b:f:he6o:");
+/*
+ * Fail if we detect an unknown argument
+ */
+if (c == '?') {
+return 1;
+}
if (c == -1) {
break;
}
@@ -472,6 +478,12 @@ static int img_check(int argc, char **argv)
fmt = NULL;
for(;;) {
c = getopt(argc, argv, "f:h");
+/*
+ * Fail if we detect an unknown argument
+ */
+if (c == '?') {
+return 1;
+}
if (c == -1) {
break;
}
@@ -550,6 +562,12 @@ static int img_commit(int argc, char **argv)
fmt = NULL;
for(;;) {
c = getopt(argc, argv, "f:h");
+/*
+ * Fail if we detect an unknown argument
+ */
+if (c == '?') {
+return 1;
+}
if (c == -1) {
break;
}
@@ -688,6 +706,12 @@ static int img_convert(int argc, char **argv)
flags = 0;
for(;;) {
c = getopt(argc, argv, "f:O:B:s:hce6o:");
+/*
+ * Fail if we detect an unknown argument
+ */
+if (c == '?') {
+return 1;
+}
if (c == -1) {
break;
}
@@ -1094,6 +1118,12 @@ static int img_info(int argc, char **argv)
fmt = NULL;
for(;;) {
c = getopt(argc, argv, "f:h");
+/*
+ * Fail if we detect an unknown argument
+ */
+if (c == '?') {
+return 1;
+}
if (c == -1) {
break;
}
@@ -1171,6 +1201,12 @@ static int img_snapshot(int argc, char **argv)
/* Parse commandline parameters */
for(;;) {
c = getopt(argc, argv, "la:c:d:h");
+/*
+ * Fail if we detect an unknown argument
+ */
+if (c == '?') {
+return 1;
+}
if (c == -1) {
break;
}
@@ -1286,6 +1322,12 @@ static int img_rebase(int argc, char **argv)
for(;;) {
c = getopt(argc, argv, "uhf:F:b:");
+/*
+ * Fail if we detect an unknown argument
+ */
+if (c == '?') {
+return 1;
+}
if (c == -1) {
break;
}
@@ -1500,6 +1542,12 @@ static int img_resize(int argc, char **argv)
fmt = NULL;
for(;;) {
c = getopt(argc, argv, "f:h");
+/*
+ * Fail if we detect an unknown argument
+ */
+if (c == '?') {
+return 1;
+}
if (c == -1) {
break;
}
--
1.7.3.2
[Qemu-devel] [PATCH 1/3] Consolidate printing of block driver options
From: Jes Sorensen
This consolidates the printing of block driver options in
print_block_option_help() which is called from both img_create() and
img_convert().
This allows for the "?" detection to be done just after the parsing of
options and the filename, instead of half way down the codepath of
these functions.
Signed-off-by: Jes Sorensen
---
qemu-img.c | 46 +-
1 files changed, 37 insertions(+), 9 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index fa77ac0..99f30b3 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -188,6 +188,32 @@ static int read_password(char *buf, int buf_size)
}
#endif
+static int print_block_option_help(const char *filename, const char *fmt)
+{
+BlockDriver *drv, *proto_drv;
+QEMUOptionParameter *create_options = NULL;
+
+/* Find driver and parse its options */
+drv = bdrv_find_format(fmt);
+if (!drv) {
+error("Unknown file format '%s'", fmt);
+return 1;
+}
+
+proto_drv = bdrv_find_protocol(filename);
+if (!proto_drv) {
+error("Unknown protocol '%s'", filename);
+return 1;
+}
+
+create_options = append_option_parameters(create_options,
+ drv->create_options);
+create_options = append_option_parameters(create_options,
+ proto_drv->create_options);
+print_option_help(create_options);
+return 0;
+}
+
static BlockDriverState *bdrv_new_open(const char *filename,
const char *fmt,
int flags)
@@ -310,6 +336,11 @@ static int img_create(int argc, char **argv)
help();
filename = argv[optind++];
+if (options && !strcmp(options, "?")) {
+ret = print_block_option_help(filename, fmt);
+goto out;
+}
+
/* Find driver and parse its options */
drv = bdrv_find_format(fmt);
if (!drv) {
@@ -328,11 +359,6 @@ static int img_create(int argc, char **argv)
create_options = append_option_parameters(create_options,
proto_drv->create_options);
-if (options && !strcmp(options, "?")) {
-print_option_help(create_options);
-goto out;
-}
-
/* Create parameter list with default values */
param = parse_option_parameters("", create_options, param);
set_option_parameter_int(param, BLOCK_OPT_SIZE, -1);
@@ -694,6 +720,11 @@ static int img_convert(int argc, char **argv)
out_filename = argv[argc - 1];
+if (options && !strcmp(options, "?")) {
+ret = print_block_option_help(out_filename, out_fmt);
+goto out2;
+}
+
if (bs_n > 1 && out_baseimg) {
error("-B makes no sense when concatenating multiple input images");
return 1;
@@ -749,10 +780,6 @@ static int img_convert(int argc, char **argv)
drv->create_options);
create_options = append_option_parameters(create_options,
proto_drv->create_options);
-if (options && !strcmp(options, "?")) {
-print_option_help(create_options);
-goto out;
-}
if (options) {
param = parse_option_parameters(options, create_options, param);
@@ -984,6 +1011,7 @@ out:
}
}
free(bs);
+out2:
if (ret) {
return 1;
}
--
1.7.3.2
[Qemu-devel] [PATCH 0/3] Cleanup qemu-img code
From: Jes Sorensen Hi, These patches moves the handling of block help printing to shared code, which allows the "?" detection to happen early in the parsing, instead of half way down img_create() and img_convert(). I would like to see this happen as I would like to pull some of the code out of img_create() and into block.c so it can be shared with qemu and qemu-img. The formatting patch is solely because the third patch wanted to change code next to the badly formatted code, and I didn't want to pollute the patch with the formatting fixed. The third patch fixes qemu-img to exit on detection of unknown options instead of continuing with a potentially wrong set of arguments. Cheers, Jes Jes Sorensen (3): Consolidate printing of block driver options Fix formatting and missing braces in qemu-img.c Fail if detecting an unknown option qemu-img.c | 171 +++ 1 files changed, 136 insertions(+), 35 deletions(-) -- 1.7.3.2
Re: [Qemu-devel] CFP: 1st International QEMU Users Forum
On 11/29/10 08:44, Jes Sorensen wrote: > On 11/26/10 15:21, Anthony Liguori wrote: >> Attaching is easier logistically but I don't know how much that helps if >> it's a full 3 days instead of just a single day. Might be worth poking >> the LF folks for some advice. >> >> But the key point is breadth, it should not be a "KVM Forum" but rather >> an Open Virtualization conference with topics as high level as cloud >> software automation and as low level as asynchronous page faults :-) > > Well that is definitely worth looking at, question is what continent > would be preferred to have it on? > > I'll have a chat with LF to see what the options are. Hi, As promised, I checked with the Linux Foundation about options, and I have popped it into the following Wiki page: http://www.linux-kvm.org/page/KVM_Forum_2011 Note that this page is purely for discussion purposes at this point, nothing has been decided yet! Things may change without warning until a decision is made and confirmed! Thanks, Jes
[Qemu-devel] [Bug 587993] Re: qemu-kvm 0.12.4+dfsg-1 from debian squeeze crashes "BUG: unable to handle kernel NULL pointer" (sym53c8xx)
Looks a duplicate of https://sourceforge.net/tracker/index.php?func=detail&aid=2042889&group_id=180599&atid=893831 Closed the SF bug, lets focus on this issue here. Jes ** Bug watch added: SourceForge.net Tracker #2042889 http://sourceforge.net/support/tracker.php?aid=2042889 -- qemu-kvm 0.12.4+dfsg-1 from debian squeeze crashes "BUG: unable to handle kernel NULL pointer" (sym53c8xx) https://bugs.launchpad.net/bugs/587993 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Incomplete Bug description: I use eucalyptus software (1.6.2) on debian squeeze with kvm 0.12.4+dfsg-1 (the same happend with 0.11.1+dfsg-1 ). Kernel 2.6.32-3-amd64. After a few days machines crash. There are no logs in host system. Guest is the same kernel and OS as host. The kvm process use 100% of cpu time. I can not even ping the guest. Everything works fine with 2.6.30-2-amd64 and 2.6.32-trunk-amd64. The problem is only with 2.6.32-3-amd64 and 2.6.32-5-amd64. Here is the log from virtual machine: [ 3577.81] sd 0:0:0:0: [sda] ABORT operation started [ 3582.816047] sd 0:0:0:0: ABORT operation timed-out. [ 3582.816781] sd 0:0:0:0: [sda] ABORT operation started [ 3587.816649] sd 0:0:0:0: ABORT operation timed-out. [ 3587.817379] sd 0:0:0:0: [sda] DEVICE RESET operation started [ 3592.816062] sd 0:0:0:0: DEVICE RESET operation timed-out. [ 3592.816882] sd 0:0:0:0: [sda] BUS RESET operation started [ 3592.820056] sym0: SCSI BUS reset detected. [ 3592.831538] sym0: SCSI BUS has been reset. [ 3592.831968] BUG: unable to handle kernel NULL pointer dereference at 0358 [ 3592.832003] IP: [] sym_int_sir+0x62f/0x14e0 [sym53c8xx] [ 3592.832003] PGD 5f73e067 PUD 5fa53067 PMD 0 [ 3592.832003] Oops: [#1] SMP [ 3592.832003] last sysfs file: /sys/devices/pci:00/:00:05.0/host0/target0:0:0/0:0:0:0/vendor [ 3592.832003] CPU 0 [ 3592.832003] Modules linked in: dm_mod openafs(P) ext2 snd_pcsp snd_pcm snd_timer serio_raw i2c_piix4 snd virtio_balloon evdev i2c_core soundcore psmouse button processor snd_page_alloc ext3 jbd mbcache sd_mod crc_t10dif ata_generic libata ide_pci_generic sym53c8xx scsi_transport_spi thermal piix uhci_hcd ehci_hcd floppy thermal_sys scsi_mod virtio_pci virtio_ring virtio e1000 ide_core usbcore nls_base [last unloaded: scsi_wait_scan] [ 3592.832003] Pid: 193, comm: scsi_eh_0 Tainted: P 2.6.32-3-amd64 #1 Bochs [ 3592.832003] RIP: 0010:[] [] sym_int_sir+0x62f/0x14e0 [sym53c8xx] [ 3592.832003] RSP: 0018:880001803cb0 EFLAGS: 00010287 [ 3592.832003] RAX: 000a RBX: 000b RCX: 5f410090 [ 3592.832003] RDX: RSI: 88005c450800 RDI: c9a5e006 [ 3592.832003] RBP: 88005f41 R08: R09: [ 3592.832003] R10: 003a R11: 813b871e R12: 88005f410090 [ 3592.832003] R13: 0084 R14: R15: 0001 [ 3592.832003] FS: () GS:88000180() knlGS: [ 3592.832003] CS: 0010 DS: 0018 ES: 0018 CR0: 8005003b [ 3592.832003] CR2: 0358 CR3: 5e269000 CR4: 06f0 [ 3592.832003] DR0: DR1: DR2: [ 3592.832003] DR3: DR6: 0ff0 DR7: 0400 [ 3592.832003] Process scsi_eh_0 (pid: 193, threadinfo 88005f6fa000, task 88005f697880) [ 3592.832003] Stack: [ 3592.832003] 88005f3fd000 0130 [ 3592.832003] <0> 88005f407710 c9a64710 ff10 81195301 [ 3592.832003] <0> 0010 00010212 880001803d18 0018 [ 3592.832003] Call Trace: [ 3592.832003] [ 3592.832003] [] ? __memcpy_toio+0x9/0x19 [ 3592.832003] [] ? sym_interrupt+0x46c/0x6a3 [sym53c8xx] [ 3592.832003] [] ? update_curr+0xa6/0x147 [ 3592.832003] [] ? sym53c8xx_intr+0x43/0x6a [sym53c8xx] [ 3592.832003] [] ? handle_IRQ_event+0x58/0x126 [ 3592.832003] [] ? handle_fasteoi_irq+0x7d/0xb5 [ 3592.832003] [] ? handle_irq+0x17/0x1d [ 3592.832003] [] ? do_IRQ+0x57/0xb6 [ 3592.832003] [] ? ret_from_intr+0x0/0x11 [ 3592.832003] [] ? __do_softirq+0x6e/0x19f [ 3592.832003] [] ? tick_dev_program_event+0x2d/0x95 [ 3592.832003] [] ? call_softirq+0x1c/0x30 [ 3592.832003] [] ? do_softirq+0x3f/0x7c [ 3592.832003] [] ? irq_exit+0x36/0x76 [ 3592.832003] [] ? smp_apic_timer_interrupt+0x87/0x95 [ 3592.832003] [] ? apic_timer_interrupt+0x13/0x20 [ 3592.832003] [ 3592.832003] [] ? delay_tsc+0x0/0x73 [ 3592.832003] [] ? sym_eh_handler+0x22e/0x2e2 [sym53c8xx] [ 3592.832003] [] ? scsi_try_bus_reset+0x50/0xd9 [scsi_mod] [ 3592.832003] [] ? scsi_eh_ready_devs+0x50c/0x781 [scsi_mod] [ 3592.832003] [] ? scsi_error_handler+0x3c1/0x5b5 [scsi_mod] [ 3592.832003] [] ? scsi_error_handler+0x0/0x5b5 [scsi_mod] [ 3592.832003] [] ? kthread+0x79/0x81 [ 3592.8320
[Qemu-devel] [Bug 680350] Re: fail to compile qemu-kvm-0.13.0
Your report is incomplete. Please provide your configure line, and also what system your are compiling on. In addition, please try building top of tree qemu with this configure line. Thanks, Jes ** Changed in: qemu Status: New => Incomplete -- fail to compile qemu-kvm-0.13.0 https://bugs.launchpad.net/bugs/680350 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Incomplete Bug description: Problem During compile with qemu-kvm-0.13.0, can't successfully make through with error: #make && make install kvm-all.o: In function `kvm_run': /home/liheyuan/code/qemu-kvm-0.13.0/qemu-kvm.c:675: undefined reference to `kvm_handle_internal_error' collect2: ld returned 1 exit status Cause The developer using an new ‘macro definition’:KVM_CAP_INTERNAL_ERROR_DATA, but fail to add it to every places it needed. Solution modify qemu-kvm.c, line:674,add the ‘macro definition’ before and after the ‘case:’, as follows #line674~~677 should be replace like this: #ifdef KVM_CAP_INTERNAL_ERROR_DATA case KVM_EXIT_INTERNAL_ERROR: kvm_handle_internal_error(env, run); r = 1; break; #endif For more details at, http://www.coder4.com/archives/1174
Re: [Qemu-devel] CFP: 1st International QEMU Users Forum
On 11/26/10 16:19, Alexander Graf wrote: > > On 26.11.2010, at 16:10, Peter Maydell wrote: > >> On 26 November 2010 14:21, Anthony Liguori wrote: >> >>> But the key point is breadth, it should not be a "KVM Forum" but rather an >>> Open Virtualization conference with topics as high level as cloud software >>> automation and as low level as asynchronous page faults :-) >> >> Does that breadth include the emulation/TCG side of qemu or only >> the virtualisation/KVM side? I'm never quite sure with titles that >> just say "Virtualization"... > > Historically, virtualization gatherings have been fairly non-tcg friendly. I > don't see why we'd have to continue that trend though. A lot of the code > actually affects both sides. IMHO it would be good to have focus on broader QEMU collaboration, including TCG. Cheers, Jes
Re: [Qemu-devel] CFP: 1st International QEMU Users Forum
On 11/26/10 15:21, Anthony Liguori wrote: > On 11/26/2010 08:15 AM, Jes Sorensen wrote: >> I would be all in favor of this! Do you want to attach it to another >> conference or totally standalone? >> > > Attaching is easier logistically but I don't know how much that helps if > it's a full 3 days instead of just a single day. Might be worth poking > the LF folks for some advice. > > But the key point is breadth, it should not be a "KVM Forum" but rather > an Open Virtualization conference with topics as high level as cloud > software automation and as low level as asynchronous page faults :-) Well that is definitely worth looking at, question is what continent would be preferred to have it on? I'll have a chat with LF to see what the options are. Cheers, Jes
Re: [Qemu-devel] CFP: 1st International QEMU Users Forum
On 11/26/10 15:13, Anthony Liguori wrote: > On 11/26/2010 07:39 AM, Alexander Graf wrote: >> >> I can certainly get in touch with the organizers, but I can't answer >> the latter question. There are usually at least ~10-20 kernel >> developers around and it's easy to get most of the German qemu people >> there too I assume (which is quite a significant number, though mostly >> kvm focused). >> > > An alternative (or maybe just addition) would be to organize a > virtualization conference next year instead of just doing another KVM > Forum. The idea would be to make it 3 days, have dedicated tracks for > management (libvirt/Open Stack?), QEMU, and KVM and Xen. > > I think as an overall community, we're large enough that we could > support a pretty sizable conference and I think everyone would benefit > from being more exposed to other aspects of the stack. I would be all in favor of this! Do you want to attach it to another conference or totally standalone? Cheers, Jes
Re: [Qemu-devel] CFP: 1st International QEMU Users Forum
On 11/26/10 12:53, Alexander Graf wrote: > > On 26.11.2010, at 08:56, Jes Sorensen wrote: >> Doing a get together somewhere in Europe really shouldn't be that hard >> to organize, however it would probably be useful to have it attached to >> a bigger conference to help with the logistics. There is going to be >> LinuxCon Europe next year, in Prague I believe, but I don't think it is >> before October. >> >> If there is interest, I am happy to talk to the LF people about getting >> a QEMU mini-conference attached to it, or we can look for something sooner? > > FOSDEM is in February and has a virtualization devroom, but I'd rather go for > something more officially Qemu. The next big thing coming to mind in Europe > is Linuxtag in Berlin. That's in May. I don't know LinuxTag that well, never been. Anyone have connection to the organizers and would it be worth having a QEMU workshop there? Cheers, Jes
Re: [Qemu-devel] [PATCH] Make SCSI HBA configurable
On 11/25/10 14:22, Paul Brook wrote: >> On 25.11.2010, at 11:59, Paul Brook wrote: >> RH needs to compile out as much as they can from the code base, because >> they state that they support everything that's compiled in. So making as >> much as possible optional is good. And I don't see why we should limit >> ourselves here. > > My second point (should be enabled by default) still applies. Your patch > removes the lsi controller from the default arm-softmmu config, which is > definitely wrong. Right I am not advocating changing the defaults, all I suggest is we make it an option to disable some of the devices. There are people running on smaller systems or dedicated systems where they know exactly which 7 devices they need and nothing more. They would be quite happy to be able to strip down QEMU to the minimum they need. Cheers, Jes
Re: [Qemu-devel] CFP: 1st International QEMU Users Forum
On 11/26/10 00:48, Alexander Graf wrote: > > On 25.11.2010, at 23:53, Andreas Färber wrote: > From the fosdem homepage: > > We would like to inform all interested parties that the call for devrooms > is running at its end. > Coming Saturday, 16 October at 23.59 the call for devrooms closes. > > So I guess this idea came too late. We will have another KVM/Kernel track > during the Chemnitzer Linux Tage next year, but that's not quite the same. > > > Alex Doing a get together somewhere in Europe really shouldn't be that hard to organize, however it would probably be useful to have it attached to a bigger conference to help with the logistics. There is going to be LinuxCon Europe next year, in Prague I believe, but I don't think it is before October. If there is interest, I am happy to talk to the LF people about getting a QEMU mini-conference attached to it, or we can look for something sooner? Cheers, Jes
[Qemu-devel] Re: [PATCH v5] virtio-9p: fix build on !CONFIG_UTIMENSAT
On 11/24/10 03:38, Hidetoshi Seto wrote: > This patch introduce a fallback mechanism for old systems that do not > support utimensat(). This fix build failure with following warnings: > > hw/virtio-9p-local.c: In function 'local_utimensat': > hw/virtio-9p-local.c:479: warning: implicit declaration of function > 'utimensat' > hw/virtio-9p-local.c:479: warning: nested extern declaration of 'utimensat' > > and: > > hw/virtio-9p.c: In function 'v9fs_setattr_post_chmod': > hw/virtio-9p.c:1410: error: 'UTIME_NOW' undeclared (first use in this > function) > hw/virtio-9p.c:1410: error: (Each undeclared identifier is reported only once > hw/virtio-9p.c:1410: error: for each function it appears in.) > hw/virtio-9p.c:1413: error: 'UTIME_OMIT' undeclared (first use in this > function) > hw/virtio-9p.c: In function 'v9fs_wstat_post_chmod': > hw/virtio-9p.c:2905: error: 'UTIME_OMIT' undeclared (first use in this > function) > > [NOTE: At this time virtio-9p is only user of utimensat(), and is available >only when host is linux and CONFIG_VIRTFS is defined. So there are >no similar warning for win32. Please provide a wrapper for win32 in >oslib-win32.c if new user really requires it.] > > v5: > - Allow fallback on runtime > - Move qemu_utimensat() to oslib-posix.c > - Rebased on latest qemu.git > v4: > - Use tv_now.tv_usec > v3: > - Use better alternative handling for UTIME_NOW/OMIT > - Move qemu_utimensat() to cutils.c > V2: > - Introduce qemu_utimensat() > > Acked-by: Chris Wright > Acked-by: M. Mohan Kumar > Signed-off-by: Hidetoshi Seto > --- > hw/virtio-9p-local.c |4 ++-- > oslib-posix.c | 48 > qemu-os-posix.h | 12 > 3 files changed, 62 insertions(+), 2 deletions(-) Hi Hidetoshi, This looks good to me! Acked-by: Jes Sorensen Cheers, Jes
Re: [Qemu-devel] [PATCH 1/1] NBD isn't used by qemu-img, so don't link qemu-img against NBD objects
On 11/22/10 16:20, Anthony Liguori wrote: > On 11/22/2010 09:10 AM, Jes Sorensen wrote: >> On 11/22/10 16:08, Anthony Liguori wrote: >>> On 11/22/2010 08:58 AM, Jes Sorensen wrote: >>>> Right, the right solution is probably to create a block driver list >>>> argument for configure, similar to what we have for the sound drivers. >>>> >>> --block-drv-whitelist= >>> >> Any idea what the difference is between 'whitelist' and 'list' in this >> context? > > Everything is built, but only the formats that are in the whitelist are > usable. Kinda defeats the purpose IMHO. It would be useful to be able to strip out the formats one doesn't want to get a slimmed down binary. Cheers, Jes
Re: [Qemu-devel] [PATCH 1/1] NBD isn't used by qemu-img, so don't link qemu-img against NBD objects
On 11/22/10 16:08, Anthony Liguori wrote: > On 11/22/2010 08:58 AM, Jes Sorensen wrote: >> On 11/22/10 15:54, Anthony Liguori wrote: >>> Using block format whitelisting should be enough to disable nbd. I >>> don't see a need for an explicit --disable-nbd option. >>> >> Right, the right solution is probably to create a block driver list >> argument for configure, similar to what we have for the sound drivers. >> > > --block-drv-whitelist= Any idea what the difference is between 'whitelist' and 'list' in this context? Jes
Re: [Qemu-devel] [PATCH] Make SCSI HBA configurable
On 11/22/10 11:15, Hannes Reinecke wrote: > > This patch introduces configuration variables > CONFIG_SCSI_LSI > CONFIG_SCSI_MEGASAS > and renames the existing CONFIG_ESP to CONFIG_SCSI_ESP. > With this the available SCSI HBAs can be configured for each > target configuration instead of compiling it in for everyone. > > Signed-off-by: Hannes Reinecke > --- > Makefile.objs|5 +++-- > default-configs/i386-softmmu.mak |2 ++ > default-configs/mips-softmmu.mak |2 +- > default-configs/mips64-softmmu.mak |2 +- > default-configs/mips64el-softmmu.mak |2 +- > default-configs/mipsel-softmmu.mak |2 +- > default-configs/ppc-softmmu.mak |2 ++ > default-configs/ppc64-softmmu.mak|2 ++ > default-configs/ppcemb-softmmu.mak |2 ++ > default-configs/sparc-softmmu.mak|2 +- > default-configs/sparc64-softmmu.mak |2 ++ > default-configs/x86_64-softmmu.mak |2 ++ > 12 files changed, 20 insertions(+), 7 deletions(-) Acked-by: Jes Sorensen With a configure flag to flip modify the list from the default it would reach perfect status :) Cheers, Jes
Re: [Qemu-devel] [PATCH 1/1] NBD isn't used by qemu-img, so don't link qemu-img against NBD objects
On 11/22/10 15:54, Anthony Liguori wrote: > On 11/22/2010 08:38 AM, Kevin Wolf wrote: >> You're free to dislike NBD as much as you want. Just compiling it out >> unconditionally and calling it a cleanup is a bit too much. ;-) >> >> A configure option for disabling NBD sounds reasonable, though I'm not >> sure what you're trying to achieve with it. It doesn't have any external >> dependencies that you could avoid this way, does it? >> > > Using block format whitelisting should be enough to disable nbd. I > don't see a need for an explicit --disable-nbd option. Right, the right solution is probably to create a block driver list argument for configure, similar to what we have for the sound drivers. Ignore my patch. Cheers, Jes
Re: [Qemu-devel] [PATCH 1/1] NBD isn't used by qemu-img, so don't link qemu-img against NBD objects
On 11/22/10 15:38, Kevin Wolf wrote: > Am 22.11.2010 15:27, schrieb Jes Sorensen: >> On 11/22/10 15:15, Kevin Wolf wrote: >> Well ok, seems a really backwards way to try and shoot yourself in the >> foot, but ok, maybe I should redo the patch to simply allow compiling >> NBD out instead. > > You're free to dislike NBD as much as you want. Just compiling it out > unconditionally and calling it a cleanup is a bit too much. ;-) > > A configure option for disabling NBD sounds reasonable, though I'm not > sure what you're trying to achieve with it. It doesn't have any external > dependencies that you could avoid this way, does it? Well it would allow us to not build qemu-nbd and reduce the size of qemu, those are wins, albeit not that big. Cheers, Jes
Re: [Qemu-devel] [PATCH 1/1] NBD isn't used by qemu-img, so don't link qemu-img against NBD objects
On 11/22/10 15:15, Kevin Wolf wrote: > Am 22.11.2010 15:08, schrieb Jes Sorensen: >> I am aware of that, but what on earth is qemu-img doing with NBD in the >> first place? Doesn't make much sense to me. > > The same as it's doing with file, host_device or http: Accessing images. > Start an NBD server (e.g. with qemu-nbd) and try something like qemu > -hda nbd:localhost. This is how you use the nbd block driver. Well ok, seems a really backwards way to try and shoot yourself in the foot, but ok, maybe I should redo the patch to simply allow compiling NBD out instead. Cheers, Jes
Re: [Qemu-devel] [PATCH 1/1] NBD isn't used by qemu-img, so don't link qemu-img against NBD objects
On 11/22/10 13:24, Kevin Wolf wrote: > Am 19.11.2010 17:30, schrieb jes.soren...@redhat.com: >> From: Jes Sorensen >> >> Signed-off-by: Jes Sorensen > > You're compiling the nbd block driver out here. This is certainly not > what you were attempting. (However, it's the only way to make it work, > because otherwise qemu-img will need the top-level nbd.o) > > qemu-img -help before this change shows: > > Supported formats: raw cow qcow vdi vmdk cloop dmg bochs vpc vvfat qcow2 > parallels nbd blkdebug sheepdog blkverify host_cdrom host_floppy > host_device file tftp ftps ftp https http > > Afterwards: > > Supported formats: raw cow qcow vdi vmdk cloop dmg bochs vpc vvfat qcow2 > parallels blkdebug sheepdog blkverify host_cdrom host_floppy host_device > file tftp ftps ftp https http I am aware of that, but what on earth is qemu-img doing with NBD in the first place? Doesn't make much sense to me. Cheers, Jes
Re: [Qemu-devel] [PATCH v3] virtio-9p: fix build on !CONFIG_UTIMENSAT
On 11/21/10 16:22, Anthony Liguori wrote: > On 11/14/2010 08:15 PM, Hidetoshi Seto wrote: >> This patch introduce a fallback mechanism for old systems that do not >> support utimensat(). This fix build failure with following warnings: >> >> hw/virtio-9p-local.c: In function 'local_utimensat': >> hw/virtio-9p-local.c:479: warning: implicit declaration of function >> 'utimensat' >> hw/virtio-9p-local.c:479: warning: nested extern declaration of >> 'utimensat' >> >> and: >> >> hw/virtio-9p.c: In function 'v9fs_setattr_post_chmod': >> hw/virtio-9p.c:1410: error: 'UTIME_NOW' undeclared (first use in this >> function) >> hw/virtio-9p.c:1410: error: (Each undeclared identifier is reported >> only once >> hw/virtio-9p.c:1410: error: for each function it appears in.) >> hw/virtio-9p.c:1413: error: 'UTIME_OMIT' undeclared (first use in this >> function) >> hw/virtio-9p.c: In function 'v9fs_wstat_post_chmod': >> hw/virtio-9p.c:2905: error: 'UTIME_OMIT' undeclared (first use in this >> function) >> >> v3: >>- Use better alternative handling for UTIME_NOW/OMIT >>- Move qemu_utimensat() to cutils.c >> V2: >>- Introduce qemu_utimensat() >> >> Signed-off-by: Hidetoshi Seto >> > > Applied. Thanks. > > Regards, > > Anthony Liguori Anthony, Did you actually apply this one? I don't see it in the git tree. However if you did, that was a mistake, the qemu_utimensat() should not have gone into cutils.c as I pointed out earlier, it is inconsistent. Cheers, Jes
Re: [Qemu-devel] [PATCH 1/1] NBD isn't used by qemu-img, so don't link qemu-img against NBD objects
On 11/20/10 18:22, Andreas Färber wrote: > Am 19.11.2010 um 17:30 schrieb jes.soren...@redhat.com: > >> From: Jes Sorensen >> >> Signed-off-by: Jes Sorensen >> --- >> Makefile |2 +- >> Makefile.objs | 12 ++-- >> 2 files changed, 11 insertions(+), 3 deletions(-) > > Tested-by: Andreas Färber > > Looks good to me and a clean build works okay. > > Any plans for a way to disable NBD build completely? There are warnings > about use of daemon() on Mac OS X and possibly Solaris, and there's > little point in building qemu-nbd if one does not use it. I think it would be worth adding as an option, however letting it default to 'on' to make sure it gets at least build tested unless a user explicitly disables it. Cheers, Jes
Re: [Qemu-devel] [PATCH 1/1] NBD isn't used by qemu-img, so don't link qemu-img against NBD objects
On 11/20/10 19:31, Stefan Hajnoczi wrote: > On Sat, Nov 20, 2010 at 6:04 PM, Andreas Färber > wrote: >> http://developer.apple.com/library/mac/#documentation/Darwin/Reference/ManPages/man3/daemon.3.html > > Deprecated in favor of using launchd. > > Removing qemu-nbd from the build because there is warning isn't a good > strategy. You may not use qemu-io either but it is always built. > That's important because otherwise it could bitrot, no one would > notice, and one day qemu-tools wouldn't work on Mac OSX at all > anymore. Instead we should fix the code that causes a warning. > > Is it cheating much to daemonize manually in qemu-nbd? ;) Maybe it would be feasible to write an os_daemonize() implementation based on launchd() on OSX?
Re: [Qemu-devel] [PATCH 16/16] megasas: LSI Megaraid SAS emulation
On 11/19/10 17:36, Alexander Graf wrote: > > On 19.11.2010, at 15:31, Jes Sorensen wrote: >> What I mean is for most other device types we allow to specify a list of >> wanted devices at the configure line, which is what I am suggesting we >> added support for for SCSI as well. > > Oh, you mean something like: > > hw-obj-$(CONFIG_MEGASAS) += megasas.o > > with respective entries in default-config/*? Yeah, makes sense. Correct, and I suggest we add that for the 53c895 driver too. Cheers, Jes
[Qemu-devel] [PATCH 1/1] NBD isn't used by qemu-img, so don't link qemu-img against NBD objects
From: Jes Sorensen Signed-off-by: Jes Sorensen --- Makefile |2 +- Makefile.objs | 12 ++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 747e47c..a503c1c 100644 --- a/Makefile +++ b/Makefile @@ -154,7 +154,7 @@ qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o: $(GENERATED_HEADERS) qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o -qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o +qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(nbd-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o diff --git a/Makefile.objs b/Makefile.objs index 23b17ce..5120e88 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -14,13 +14,13 @@ oslib-obj-$(CONFIG_POSIX) += oslib-posix.o # block-obj-y is code used by both qemu system emulation and qemu-img block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o -block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o +block-obj-y += block.o aio.o aes.o qemu-config.o block-obj-$(CONFIG_POSIX) += posix-aio-compat.o block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o block-nested-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat.o block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o -block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o +block-nested-y += parallels.o blkdebug.o sheepdog.o blkverify.o block-nested-$(CONFIG_WIN32) += raw-win32.o block-nested-$(CONFIG_POSIX) += raw-posix.o block-nested-$(CONFIG_CURL) += curl.o @@ -45,6 +45,13 @@ net-obj-y += $(addprefix net/, $(net-nested-y)) fsdev-nested-$(CONFIG_VIRTFS) = qemu-fsdev.o fsdev-obj-$(CONFIG_VIRTFS) += $(addprefix fsdev/, $(fsdev-nested-y)) +### +# nbd-obj-y is code used by both qemu and qemu-nbd + +nbd-obj-y = nbd.o +nbd-nested-y = nbd.o +nbd-obj-y += $(addprefix block/, $(nbd-nested-y)) + ## # libqemu_common.a: Target independent part of system emulation. The # long term path is to suppress *all* target specific code in case of @@ -53,6 +60,7 @@ fsdev-obj-$(CONFIG_VIRTFS) += $(addprefix fsdev/, $(fsdev-nested-y)) common-obj-y = $(block-obj-y) blockdev.o common-obj-y += $(net-obj-y) +common-obj-y += $(nbd-obj-y) common-obj-y += $(qobject-obj-y) common-obj-$(CONFIG_LINUX) += $(fsdev-obj-$(CONFIG_LINUX)) common-obj-y += readline.o console.o cursor.o async.o qemu-error.o -- 1.7.3.2
Re: [Qemu-devel] [PATCH 16/16] megasas: LSI Megaraid SAS emulation
On 11/19/10 15:06, Markus Armbruster wrote: >> Only comment, as you are adding another SCSI driver, maybe it's time to >> make the driver selection configurable, rather than hard coding the build? > > What do you mean by that? > > We hardcode lsi53c895a in two places where we really mean "default SCSI > controller type": pc_pci_device_init(), which applies to "-drive > if=scsi", and qemu_pci_hot_add_storage(), which applies to "pci_add > storage if=scsi". Not sure making that default configurable is worth > it. > > If you want more control than -drive and pci_add provice, use -device > and device_add. What I mean is for most other device types we allow to specify a list of wanted devices at the configure line, which is what I am suggesting we added support for for SCSI as well. Cheers, Jes
Re: [Qemu-devel] [PATCH 16/16] megasas: LSI Megaraid SAS emulation
On 11/18/10 15:47, Hannes Reinecke wrote: > This patch adds an emulation for the LSI Megaraid SAS HBA. > > Signed-off-by: Hannes Reinecke > --- > Makefile.objs |2 +- > hw/megasas.c | 1826 > + > hw/mfi.h | 1197 + > hw/pci_ids.h |2 + > hw/scsi.h |1 + > 5 files changed, 3027 insertions(+), 1 deletions(-) > create mode 100644 hw/megasas.c > create mode 100644 hw/mfi.h > > diff --git a/Makefile.objs b/Makefile.objs > index 15569af..54c6e02 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -231,7 +231,7 @@ hw-obj-$(CONFIG_IDE_MACIO) += ide/macio.o > hw-obj-$(CONFIG_IDE_VIA) += ide/via.o > > # SCSI layer > -hw-obj-y += lsi53c895a.o > +hw-obj-y += lsi53c895a.o megasas.o > hw-obj-$(CONFIG_ESP) += esp.o > > hw-obj-y += dma-helpers.o sysbus.o isa-bus.o Just had a look through your patches and have to say nice work. Haven't tested it though, but looks like a good step in the right direction. Only comment, as you are adding another SCSI driver, maybe it's time to make the driver selection configurable, rather than hard coding the build? Cheers, Jes
Re: [Qemu-devel] [RFC][PATCH v3 11/21] virtproxy: add handler for control packet
On 11/18/10 17:18, Michael Roth wrote:
> On 11/18/2010 05:35 AM, Jes Sorensen wrote:
>> You should never have two closing braces in the same column like this -
>> something is wrong with the formatting.
>
> That's from using a block for the case
> switch () {
> case: {
> ...
> }
> }
>
> Alternative is to indent the "case:", which is "right" I think, but
> aligning those with switch() seems to be pretty standard to conserve space.
Why do you need braces around the case: { } ? That is not normally used
throughout the code.
Cheers,
Jes
Re: [Qemu-devel] [RFC][PATCH v3 01/21] virtproxy: base data structures and constants
On 11/18/10 16:56, Anthony Liguori wrote: > On 11/18/2010 09:51 AM, Jes Sorensen wrote: >>> Yes, I think Jes was just looking for an excuse to say "typedefmeharder" >>> :-) >>> >> Actually typedefs are not listed as a must do thing in CODING_STYLE, >> fortunately! > > That's a bug in CODING_STYLE. > > typedefing structures is one of the core characteristics of QEMU coding > style. And here I was hoping that we could at least keep basic sanity in the coding style document :( typedefs are just plain wrong! Jes
Re: [Qemu-devel] [RFC][PATCH v3 01/21] virtproxy: base data structures and constants
On 11/18/10 16:41, Anthony Liguori wrote:
> On 11/18/2010 09:35 AM, Michael Roth wrote:
+/* listening fd, one for each service we're forwarding to remote
end */
+typedef struct VPOForward {
+VPDriver *drv;
+int listen_fd;
+char service_id[VP_SERVICE_ID_LEN];
+QLIST_ENTRY(VPOForward) next;
+} VPOForward;
>>>
>>> I am really not a fan of the typedefmeharder approach you are taking in
>>> here, but others may disagree with me.
>>
>> Isn't typedef'ing structured types part of the qemu coding style
>> guidelines?
>
> Yes, I think Jes was just looking for an excuse to say "typedefmeharder"
> :-)
Actually typedefs are not listed as a must do thing in CODING_STYLE,
fortunately! It's just a really bad habit that is applied all over the
place in QEMU :(
http://www.linuxjournal.com/article/5780?page=0,2
search for typedef, for a lot of good reasoning why we shouldn't do this.
Jes
Re: [Qemu-devel] [RFC][PATCH v4 09/18] virtagent: add va_shutdown RPC
On 11/18/10 16:35, Anthony Liguori wrote: > On 11/18/2010 08:17 AM, Jes Sorensen wrote: >> On 11/16/10 17:01, Michael Roth wrote: >> >>> RPC to initiate guest shutdown/reboot/powerdown >>> >> Do we really need this? After all those events can be passed down via >> ACPI? >> > > Reboot can't be passed by ACPI. This is why virDomainReboot is still > not functional in KVM. Every other hypervisor does something identical > to this series. > > And a hypervisor initiated shutdown ought to be treated differently than > an ACPI shutdown. Namely, it should correspond to an immediate shut > down vs. prompting the user. > > Not all OSes response to ACPI shutdown either FWIW. Ok, fair enough. I just would like to avoid duplicating functionality if we don't have to. Cheers, Jes
Re: [Qemu-devel] [RFC][PATCH v4 17/18] virtagent: qemu-vp, va_send_hello() on startup
On 11/16/10 17:01, Michael Roth wrote: > Make the hello call on guest agent startup so QEMU can do whatever init > it needs (currently, capabilities negotiation). Temporarilly commented > due to this tending to induce a virtio bug in RHEL 6.0. As a result > capabilities negotiation must be invoked manually from QEMU via the > agent_capabilities monitor command. > > Signed-off-by: Michael Roth > --- > qemu-vp.c |2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/qemu-vp.c b/qemu-vp.c > index 38959e5..b8af513 100644 > --- a/qemu-vp.c > +++ b/qemu-vp.c > @@ -580,6 +580,8 @@ int main(int argc, char **argv) > errx(EXIT_FAILURE, > "error initializing guest agent"); > } > +/* tell the host the agent is running */ > +//va_send_hello(); Ehm, not much point adding a call that is commented out :) Cheers, Jes
Re: [Qemu-devel] [RFC][PATCH v4 14/18] virtagent: add client capabilities init function
On 11/16/10 17:01, Michael Roth wrote:
> Non-monitor version of agent_capabilities monitor function. This is
> called by the local RPC server when it gets a "hello" from the guest
> agent to re-negotiate guest agent capabilities.
>
> Signed-off-by: Michael Roth
> ---
> virtagent.c | 34 ++
> virtagent.h |1 +
> 2 files changed, 35 insertions(+), 0 deletions(-)
>
> diff --git a/virtagent.c b/virtagent.c
> index e0f7f99..4ec1b42 100644
> --- a/virtagent.c
> +++ b/virtagent.c
> @@ -694,3 +694,37 @@ int do_agent_capabilities(Monitor *mon, const QDict
> *mon_params,
>
> return 0;
> }
> +
> +/* non-HMP/QMP RPC client functions */
> +
> +int va_client_init_capabilities(void)
> +{
> +xmlrpc_env env;
> +xmlrpc_value *params;
> +VARPCData *rpc_data;
> +int ret;
> +
> +xmlrpc_env_init(&env);
> +
> +params = xmlrpc_build_value(&env, "()");
> +if (rpc_has_error(&env)) {
> +return -1;
> +}
> +
> +rpc_data = qemu_mallocz(sizeof(VARPCData));
> +rpc_data->cb = do_agent_capabilities_cb;
> +rpc_data->mon_cb = NULL;
> +rpc_data->mon_data = NULL;
> +
> +ret = rpc_execute(&env, "system.listMethods", params, rpc_data);
> +if (ret == -EREMOTE) {
> +LOG("RPC Failed (%i): %s\n", env.fault_code,
> +env.fault_string);
> +return -1;
> +} else if (ret == -1) {
> +LOG("RPC communication error\n");
> +return -1;
> +}
One of many examples that would have benefited from having a utility
function doing most of the work here.
Cheers,
Jes
Re: [Qemu-devel] [RFC][PATCH v4 10/18] virtagent: add agent_shutdown monitor command
On 11/16/10 17:01, Michael Roth wrote:
> Provide monitor command to initiate guest shutdown/reboot/powerdown
> +int do_agent_shutdown(Monitor *mon, const QDict *mon_params,
> + MonitorCompletion cb, void *opaque)
> +{
> +xmlrpc_env env;
> +xmlrpc_value *params;
> +VARPCData *rpc_data;
> +const char *shutdown_type;
> +int ret;
> +
> +TRACE("called");
> +
> +xmlrpc_env_init(&env);
> +shutdown_type = qdict_get_str(mon_params, "shutdown_type");
> +params = xmlrpc_build_value(&env, "(s)", shutdown_type);
> +if (rpc_has_error(&env)) {
> +return -1;
> +}
> +
> +rpc_data = qemu_mallocz(sizeof(VARPCData));
> +rpc_data->cb = do_agent_shutdown_cb;
> +rpc_data->mon_cb = cb;
> +rpc_data->mon_data = opaque;
> +
> +ret = rpc_execute(&env, "va_shutdown", params, rpc_data);
> +if (ret == -EREMOTE) {
> +monitor_printf(mon, "RPC Failed (%i): %s\n", env.fault_code,
> + env.fault_string);
> +return -1;
> +} else if (ret == -1) {
> +monitor_printf(mon, "RPC communication error\n");
> +return -1;
> +}
I would think you could put a lot of this into a utility function
instead of having it open coded for each command you want to support?
Cheers,
Jes
> +
> +return 0;
> +}
> diff --git a/virtagent.h b/virtagent.h
> index c077582..96c6260 100644
> --- a/virtagent.h
> +++ b/virtagent.h
> @@ -29,5 +29,7 @@ int do_agent_viewfile(Monitor *mon, const QDict *mon_params,
> void do_agent_viewdmesg_print(Monitor *mon, const QObject *qobject);
> int do_agent_viewdmesg(Monitor *mon, const QDict *mon_params,
>MonitorCompletion cb, void *opaque);
> +int do_agent_shutdown(Monitor *mon, const QDict *mon_params,
> + MonitorCompletion cb, void *opaque);
>
> #endif /* VIRTAGENT_H */
Re: [Qemu-devel] [RFC][PATCH v4 09/18] virtagent: add va_shutdown RPC
On 11/16/10 17:01, Michael Roth wrote: > RPC to initiate guest shutdown/reboot/powerdown Do we really need this? After all those events can be passed down via ACPI? Cheers, Jes
Re: [Qemu-devel] [RFC][PATCH v4 06/18] virtagent: add agent_viewfile command
On 11/16/10 17:01, Michael Roth wrote: > Utilize the getfile RPC to provide a means to view text files in the > guest. Getfile can handle binary files as well but we don't advertise > that here due to the special handling requiring to store it and provide > it back to the user (base64 encoding it for instance). Hence the > potentially confusing "viewfile" as opposed to "getfile". > > Signed-off-by: Michael Roth > This one is full of -1 returns again :( Cheers, Jes
Re: [Qemu-devel] [RFC][PATCH v4 04/18] virtagent: base RPC client definitions
On 11/16/10 17:01, Michael Roth wrote:
> diff --git a/monitor.c b/monitor.c
> index 8cee35d..cb81cd7 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -42,6 +42,7 @@
> #include "audio/audio.h"
> #include "disas.h"
> #include "balloon.h"
> +#include "virtagent.h"
> #include "qemu-timer.h"
> #include "migration.h"
> #include "kvm.h"
You are adding an include here without modifying any code to actually
use something from this file.
> +static int va_client_ready(void)
> +{
> +if (client_state != NULL && client_state->vp != NULL
> +&& client_state->socket_path != NULL) {
Please put the && up on the previous line, makes it easier for a reader
to notice that the next line is part of the first portion.
> +return 0;
> +}
> +
> +return -1;
> +}
-EAGAIN? -EBUSY?
> +static void va_set_capabilities(QList *qlist)
> +{
> +TRACE("called");
> +
> +if (client_state == NULL) {
> +LOG("client is uninitialized, unable to set capabilities");
> +return;
> +}
Why no error value returned here?
> +int va_client_init(VPDriver *vp_drv, bool is_host)
> +{
> +const char *service_id, *path;
> +QemuOpts *opts;
> +int fd, ret;
> +
> +if (client_state) {
> +LOG("virtagent client already initialized");
> +return -1;
> +}
-1 again :(
> +/* setup listening socket to forward connections over */
> +opts = qemu_opts_create(qemu_find_opts("net"), "va_client_opts", 0);
> +qemu_opt_set(opts, "path", path);
> +fd = unix_listen_opts(opts);
> +qemu_opts_del(opts);
> +if (fd < 0) {
> +LOG("error setting up listening socket");
> +goto out_bad;
> +}
> +
> +/* tell virtproxy to forward connections to this socket to
> + * virtagent service on other end
> + */
> +ret = vp_set_oforward(vp_drv, fd, service_id);
> +if (ret < 0) {
> +LOG("error setting up virtproxy iforward");
> +goto out_bad;
> +}
> +
> +return 0;
> +out_bad:
> +qemu_free(client_state);
> +client_state = NULL;
> +return -1;
> +}
You know why you ended up here, please pass that information up the stack.
> +static int rpc_has_error(xmlrpc_env *env)
> +{
> +if (env->fault_occurred) {
> +LOG("An RPC error has occurred (%i): %s\n", env->fault_code,
> env->fault_string);
> +//qerror_report(QERR_RPC_FAILED, env->fault_code, env->fault_string);
> +return -1;
> +}
> +return 0;
> +}
-1 again
> +/*
> + * Get a connected socket that can be used to make an RPC call
> + * This interface will eventually return the connected virtproxy socket for
> the
> + * virt-agent channel
> + */
> +static int get_transport_fd(void)
> +{
> +/* TODO: eventually this will need a path that is unique to other
> + * instances of qemu-vp/qemu. for the integrated qemu-vp we should
> + * explore the possiblity of not requiring a unix socket under the
> + * covers, as well as having client init code set up the oforward
> + * for the service rather than qemu-vp
> + */
> +int ret;
> +int fd = unix_connect(client_state->socket_path);
> +if (fd < 0) {
> +LOG("failed to connect to virtagent service");
> +}
> +ret = fcntl(fd, F_GETFL);
> +ret = fcntl(fd, F_SETFL, ret | O_NONBLOCK);
> +return fd;
> +}
You are forgetting to check the return from fcntl() for errors.
> +static int rpc_execute(xmlrpc_env *const env, const char *function,
> + xmlrpc_value *params, VARPCData *rpc_data)
> +{
> +xmlrpc_mem_block *call_xml;
> +int fd, ret;
> +
> +ret = va_client_ready();
> +if (ret < 0) {
> +LOG("client in uninitialized state, unable to execute RPC");
> +ret = -1;
> +goto out;
> +}
> +
> +if (!va_has_capability(function)) {
> +LOG("guest agent does not have required capability");
> +ret = -1;
> +goto out;
> +}
> +
> +fd = get_transport_fd();
> +if (fd < 0) {
> +LOG("invalid fd");
> +ret = -1;
> +goto out;
> +}
You just got a proper error value back, why replace it with -1? It is
all over this function.
> diff --git a/virtagent.h b/virtagent.h
> new file mode 100644
> index 000..53efa29
> --- /dev/null
> +++ b/virtagent.h
> +#define GUEST_AGENT_PATH_CLIENT "/tmp/virtagent-guest-client.sock"
> +#define HOST_AGENT_PATH_CLIENT "/tmp/virtagent-host-client.sock"
> +#define VA_MAX_CHUNK_SIZE 4096 /* max bytes at a time for get/send file */
Config file please!
Jes
Re: [Qemu-devel] [RFC][PATCH v4 03/18] virtagent: qemu-vp, integrate virtagent server
On 11/16/10 17:01, Michael Roth wrote:
> +bool enable_virtagent;
>
> chr->opaque = drv;
> chr->chr_write = vp_chr_write;
> @@ -2025,9 +2029,31 @@ static CharDriverState
> *qemu_chr_open_virtproxy(QemuOpts *opts)
> /* parse socket forwarding options */
> qemu_opt_foreach(opts, vp_init_forwards, drv, 1);
>
> +/* add forwarding options to enable virtagent server */
> +enable_virtagent = qemu_opt_get_bool(opts, "virtagent", 0);
int qemu_opt_get_bool(QemuOpts *opts, const char *name, int defval)
Sorry qemu_opt_get_bool() actually returns an int.
> diff --git a/qemu-vp.c b/qemu-vp.c
> index cfd2a69..38959e5 100644
> --- a/qemu-vp.c
> +++ b/qemu-vp.c
> @@ -37,6 +37,8 @@
> #include "qemu-option.h"
> #include "qemu_socket.h"
> #include "virtproxy.h"
> +#include "virtagent.h"
> +#include "virtagent-daemon.h"
>
> static bool verbose_enabled = 0;
You don't need to initialize global variables to zero, the compiler does
that.
> +static int init_agent(const VPData *agent_iforward) {
> +QemuOpts *opts = agent_iforward->opts;
> +VPDriver *drv;
> +int ret, index;
> +
> +INFO("initializing agent...");
> +if (verbose_enabled) {
> +qemu_opts_print(opts, NULL);
> +}
> +
> +index = qemu_opt_get_number(agent_iforward->opts, "index", 0);
> +drv = get_channel_drv(index);
> +if (drv == NULL) {
> +warnx("unable to find channel with index: %d", index);
> +goto err;
> +}
> +
> +/* outbound RPCs */
> +ret = va_client_init(drv, false);
> +if (ret) {
> +warnx("error starting RPC server");
> +goto err;
> +}
> +
> +/* start guest RPC server */
> +ret = va_server_init(drv, false);
> +if (ret != 0) {
> +warnx("error starting RPC server");
> +goto err;
> +}
> +
> +return 0;
> +
> +err:
> +return -1;
> +}
Please set appropriate error codes and return something meaningful
instead of just -1.
Cheers,
Jes
Re: [Qemu-devel] [RFC][PATCH v4 02/18] virtagent: base definitions for host/guest RPC server
On 11/16/10 17:01, Michael Roth wrote:
> +#include
> +#include "qemu_socket.h"
> +#include "virtagent-daemon.h"
> +#include "virtagent-common.h"
> +#include "virtagent.h"
> +
> +static bool va_enable_syslog = false; /* enable syslog'ing of RPCs */
> +
> +#define SLOG(msg, ...) do { \
> +char msg_buf[1024]; \
> +if (!va_enable_syslog) { \
> +break; \
> +} \
> +sprintf(msg_buf, msg, ## __VA_ARGS__); \
> +syslog(LOG_INFO, "virtagent, %s", msg_buf); \
> +} while(0)
You have a potential buffer overflow here, s/sprintf/snprintf/
> +#include "virtproxy.h"
> +
> +#define GUEST_AGENT_SERVICE_ID "virtagent"
> +#define GUEST_AGENT_PATH "/tmp/virtagent-guest.sock"
> +#define HOST_AGENT_SERVICE_ID "virtagent-host"
> +#define HOST_AGENT_PATH "/tmp/virtagent-host.sock"
> +#define VA_GETFILE_MAX 1 << 30
> +#define VA_FILEBUF_LEN 16384
> +
> +int va_server_init(VPDriver *vp_drv, bool is_host);
More stuff which I think should go into a config file.
Jes
[Qemu-devel] [Bug 676029] Re: Silently fail with wrong vde socket dir
Given that you know what the problem is, it would probably have been
faster to post a patch than just updating the bug and marking it
confirmed
--
Silently fail with wrong vde socket dir
https://bugs.launchpad.net/bugs/676029
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
Status in QEMU: Confirmed
Bug description:
Hi,
Using qemu 0.12.5, kvm silently fail with exit code 1 when using -net vde and a
wrong path for sock. Actually, the sock option is mean to be the socket dir of
the vde_switch, not the socket itself.
With -net vde,sock=/var/run/vde/vde0/ctl , strace ends with the following
messages :
connect(7, {sa_family=AF_FILE, path="/var/run/vde/vde0/ctl/ctl"}, 110) = -1
ENOTDIR (Not a directory)
close(7)= 0
close(8)= 0
exit_group(1) = ?
root ~#
Please add a meaningful message.
Regards,
Étienne
Re: [Qemu-devel] [RFC][PATCH v4 01/18] virtagent: add common rpc transport defs
On 11/16/10 17:01, Michael Roth wrote:
> +#define DEBUG_VA
> +
> +#ifdef DEBUG_VA
> +#define TRACE(msg, ...) do { \
> +fprintf(stderr, "%s:%s():L%d: " msg "\n", \
> +__FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \
> +} while(0)
> +#else
> +#define TRACE(msg, ...) \
> +do { } while (0)
> +#endif
> +
> +#define LOG(msg, ...) do { \
> +fprintf(stderr, "%s:%s(): " msg "\n", \
> +__FILE__, __FUNCTION__, ## __VA_ARGS__); \
> +} while(0)
I am sure I saw those macros in a couple of other places in the tree
recently :)
> +#define TADDR "127.0.0.1:8080"
> +#define URL "http://localhost:8080/RPC2";
Rather than relying on hard coded addresses for this, how about moving
it to a config file?
Cheers,
Jes
Re: [Qemu-devel] [RFC][PATCH v4 00/18] virtagent: host/guest RPC communication agent
On 11/16/10 17:01, Michael Roth wrote: > This set of patches is meant to be applied on top of the recently submitted > Virtproxy v3 patchset. It can also be obtained at: > > git://repo.or.cz/qemu/mdroth.git virtproxy_v3 > > OVERVIEW: > > There are a wide range of use cases motivating the need for a guest agent of > some sort to extend the functionality/usability/control offered by QEMU. Some > examples include graceful guest shutdown/reboot and notifications thereof, > copy/paste syncing between host/guest, guest statistics gathering, file > access, etc. > > Ideally these would all be served by a single, easilly extensible agent that > can be deployed in a wide range of guests. Virtagent is an XMLRPC server, > integrated into QEMU and the Virtproxy guest daemon, aimed at providing this > type of functionality. > > NOTE: The guest agent can potentially be implemented independently of > virtproxy depending on the feedback, we simply make use of it to provide an > abstraction from the actual transport layer (ISA vs. Virtio serial) and use > it's multiplexing capabilities to avoid having to dedicate 2 isa/virtio > serial ports to the virtagent service. Please evaluate these patches as being > seperate from virtproxy. > Michael, On a general note, while the protocol stuff probably should go into QEMU, then I don't see the actual agent belonging in the QEMU tree. If you put it all together, you are less likely other agents are willing to adopt virtproxy. Cheers, Jes
Re: [Qemu-devel] [RFC][PATCH v3 18/21] virtproxy: qemu integration, add virtproxy chardev
On 11/16/10 02:16, Michael Roth wrote:
> This allows us to create a virtproxy instance via a chardev. It can now
> be created with something like:
>
> qemu -chardev virtproxy,id=vp1 \
> -device virtio-serial \
> -device virtserialport,chardev=vp1
>
> In the future the ability to add oforwards/iforwards in the command-line
> invocation and the monitor will be added. For now we leave it to users
> of virtproxy (currently only virtagent) to set up the forwarding
> sockets/ports they need via direct virtproxy API calls.
>
> Signed-off-by: Michael Roth
> ---
> qemu-char.c | 130
> +
> qemu-config.c |6 +++
> 2 files changed, 136 insertions(+), 0 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 88997f9..bc7925c 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -1902,6 +1902,135 @@ return_err:
> }
>
> /***/
> +/* Virtproxy chardev driver */
> +
> +#include "virtproxy.h"
> +
> +static int vp_init_oforward(VPDriver *drv, QemuOpts *opts)
> +{
> +int ret, fd;
> +const char *service_id;
> +
> +if (qemu_opt_get(opts, "host") != NULL) {
> +fd = inet_listen_opts(opts, 0);
> +} else if (qemu_opt_get(opts, "path") != NULL) {
> +fd = unix_listen_opts(opts);
> +} else {
> +fprintf(stderr, "unable to find listening socket host/addr info");
> +return -1;
-1 again
Cheers,
Jes
Re: [Qemu-devel] [RFC][PATCH v3 17/21] virtproxy: add virtproxy-builtin.c for compat defs
On 11/16/10 02:16, Michael Roth wrote: > Virtproxy relies on routines defined within qemu-vp which mirror various > i/o related operations in qemu to provide similar functionality for the > guest daemon. When building virtproxy as part of qemu rather than > qemu-vp we need these definitions to provide those functions in terms of > the original qemu functions. > > Signed-off-by: Michael Roth > --- > virtproxy-builtin.c | 38 ++ > 1 files changed, 38 insertions(+), 0 deletions(-) > create mode 100644 virtproxy-builtin.c > > diff --git a/virtproxy-builtin.c b/virtproxy-builtin.c > new file mode 100644 > index 000..71fc5bc > --- /dev/null > +++ b/virtproxy-builtin.c > @@ -0,0 +1,38 @@ > +/* > + * virt-proxy - host/guest communication layer builtin definitions > + * > + * Copyright IBM Corp. 2010 > + * > + * Authors: > + * Adam Litke > + * Michael Roth > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +/* the following are functions we define in terms of qemu when linked > + * against qemu/vl.c. these will be added on an as-needed basis > + */ > + > +#include "qemu-char.h" > +#include "qemu_socket.h" > +#include "virtproxy.h" > + > +int vp_set_fd_handler(int fd, > +IOHandler *fd_read, > +IOHandler *fd_write, > +void *opaque) odd alignment again. Jes
Re: [Qemu-devel] [RFC][PATCH v3 15/21] virtproxy: add read handler for proxied connections
On 11/16/10 02:16, Michael Roth wrote:
> +/* read handler for proxied connections */
> +static void vp_conn_read(void *opaque)
> +{
> +VPConn *conn = opaque;
> +VPDriver *drv = conn->drv;
> +VPPacket pkt;
> +char buf[VP_CONN_DATA_LEN];
> +int fd, count, ret;
> +bool client;
> +
> +TRACE("called with opaque: %p, drv: %p", opaque, drv);
> +
> +if (conn->state != VP_STATE_CONNECTED) {
> +LOG("invalid connection state");
> +return;
> +}
A read handler which doesn't pass back error values in any shape or form
does seem a little weird to me?
Cheers,
Jes
Re: [Qemu-devel] [RFC][PATCH v3 16/21] virtproxy: add option parser helper vp_parse()
On 11/16/10 02:16, Michael Roth wrote:
> +/* utility function to parse iforward/oforward options for qemu-vp
> + * or virtproxy chardev and put them into QemuOpts
> + */
> +int vp_parse(QemuOpts *opts, const char *str, bool is_channel)
> +{
> +/* TODO: use VP_SERVICE_ID_LEN, bring it into virtproxy.h */
> +char service_id[32];
> +char channel_method[32];
> +char index[10];
> +char *addr;
> +char port[33];
> +int pos, ret;
> +
> +if (is_channel == false) {
> +/* parse service id */
> +ret = sscanf(str,"%32[^:]:%n",service_id,&pos);
> +if (ret != 1) {
> +LOG("error parsing service id");
> +return -1;
-EINVAL seems more useful than -1
Cheers,
Jes
Re: [Qemu-devel] [RFC][PATCH v3 11/21] virtproxy: add handler for control packet
On 11/16/10 02:16, Michael Roth wrote:
> Process control packets coming in over the channel. This entails setting
> up/tearing down connections to local services initiated from the other
> end of the channel.
>
> Signed-off-by: Michael Roth
> ---
> virtproxy.c | 154
> +++
> 1 files changed, 154 insertions(+), 0 deletions(-)
[snip]
> +
> +qemu_opts_print(iforward->socket_opts, NULL);
> +if (qemu_opt_get(iforward->socket_opts, "host") != NULL) {
> +server_fd = inet_connect_opts(iforward->socket_opts);
> +} else if (qemu_opt_get(iforward->socket_opts, "path") != NULL) {
> +server_fd = unix_connect_opts(iforward->socket_opts);
> +} else {
> +LOG("unable to find listening socket host/addr info");
> +return -1;
> +}
This patch is a perfect example of why -1 as an error message is suboptimal.
> +closesocket(fd);
> +vp_set_fd_handler(fd, NULL, NULL, conn);
> +QLIST_REMOVE(conn, next);
> +qemu_free(conn);
> +break;
> +}
> +}
You should never have two closing braces in the same column like this -
something is wrong with the formatting.
Cheers,
Jes
Re: [Qemu-devel] [RFC][PATCH v3 10/21] virtproxy: add handler for data packets
On 11/16/10 02:16, Michael Roth wrote:
> +if (pkt->type == VP_PKT_CLIENT) {
> +TRACE("recieved client packet, client fd: %d, server fd: %d",
> + pkt->payload.proxied.client_fd,
> pkt->payload.proxied.server_fd);
> +fd = pkt->payload.proxied.server_fd;
> +} else if (pkt->type == VP_PKT_SERVER) {
> +TRACE("recieved server packet, client fd: %d, server fd: %d",
> + pkt->payload.proxied.client_fd,
> pkt->payload.proxied.server_fd);
> +fd = pkt->payload.proxied.client_fd;
> +} else {
> +TRACE("unknown packet type");
> +return -1;
> +}
-1 isn't a very friendly error value to pass up the stack.
> +/* TODO: proxied in non-blocking mode can causes us to spin here
> + * for slow servers/clients. need to use write()'s and maintain
> + * a per-conn write queue that we clear out before sending any
> + * more data to the fd
> + */
> +ret = vp_send_all(fd, (void *)pkt->payload.proxied.data,
> +pkt->payload.proxied.bytes);
> +if (ret == -1) {
> +LOG("error sending data over channel");
> +return -1;
> +} else if (ret != pkt->payload.proxied.bytes) {
> +TRACE("buffer full?");
> +return -1;
> +}
Again here, please pass the error codes up the stack so we can later
handle them appropriately.
Cheers,
Jes
Re: [Qemu-devel] [RFC][PATCH v3 03/21] virtproxy: add debug functions for virtproxy core
On 11/16/10 02:15, Michael Roth wrote:
> Signed-off-by: Michael Roth
> ---
> virtproxy.c | 17 +
> 1 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/virtproxy.c b/virtproxy.c
> index 8f18d83..3686c77 100644
> --- a/virtproxy.c
> +++ b/virtproxy.c
> @@ -13,6 +13,23 @@
>
> #include "virtproxy.h"
>
> +#define DEBUG_VP
> +
> +#ifdef DEBUG_VP
> +#define TRACE(msg, ...) do { \
> +fprintf(stderr, "%s:%s():L%d: " msg "\n", \
> +__FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \
> +} while(0)
> +#else
> +#define TRACE(msg, ...) \
> +do { } while (0)
> +#endif
> +
> +#define LOG(msg, ...) do { \
> +fprintf(stderr, "%s:%s(): " msg "\n", \
> +__FILE__, __FUNCTION__, ## __VA_ARGS__); \
> +} while(0)
> +
I wonder if it wouldn't make sense to do this in a more generic way and
stick it in a header file. This type of debug code seems to show up
repeatedly all over the place.
Cheers,
Jes
Re: [Qemu-devel] [RFC][PATCH v3 01/21] virtproxy: base data structures and constants
On 11/16/10 02:15, Michael Roth wrote:
> Signed-off-by: Michael Roth
> ---
> virtproxy.c | 136
> +++
> virtproxy.h | 34 +++
> 2 files changed, 170 insertions(+), 0 deletions(-)
> create mode 100644 virtproxy.c
> create mode 100644 virtproxy.h
>
> diff --git a/virtproxy.c b/virtproxy.c
> new file mode 100644
> index 000..8f18d83
> --- /dev/null
> +++ b/virtproxy.c
> @@ -0,0 +1,136 @@
> +/*
> + * virt-proxy - host/guest communication layer
> + *
> + * Copyright IBM Corp. 2010
> + *
> + * Authors:
> + * Michael Roth
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "virtproxy.h"
> +
> +#define VP_SERVICE_ID_LEN 32/* max length of service id string */
> +#define VP_PKT_DATA_LEN 1024/* max proxied bytes per VPPacket */
> +#define VP_CONN_DATA_LEN 1024 /* max bytes conns can send at a time */
> +#define VP_MAGIC 0x1F374059
> +
> +/* listening fd, one for each service we're forwarding to remote end */
> +typedef struct VPOForward {
> +VPDriver *drv;
> +int listen_fd;
> +char service_id[VP_SERVICE_ID_LEN];
> +QLIST_ENTRY(VPOForward) next;
> +} VPOForward;
I am really not a fan of the typedefmeharder approach you are taking in
here, but others may disagree with me.
Cheers,
Jes
Re: [Qemu-devel] [RFC][PATCH v3 02/21] virtproxy: qemu-vp, standalone daemon skeleton
On 11/16/10 02:15, Michael Roth wrote:
> Daemon to be run in guest, or on host in standalone mode.
> (re-)implements some qemu utility functions used by core virtproxy.c
> code via wrapper functions. For built-in virtproxy code we will define
> these wrapper functions in terms of qemu's built-in implementations.
>
> Main logic will come in a later patch.
>
> Signed-off-by: Michael Roth
Hi Michael,
A couple of comments:
> +/* mirror qemu I/O-related code for standalone daemon */
> +typedef struct IOHandlerRecord {
> +int fd;
> +IOCanReadHandler *fd_read_poll;
> +IOHandler *fd_read;
> +IOHandler *fd_write;
> +int deleted;
> +void *opaque;
> +/* temporary data */
> +struct pollfd *ufd;
> +QLIST_ENTRY(IOHandlerRecord) next;
> +} IOHandlerRecord;
Please move this to a header file. Any chance you could avoid some of
all those ugly typedefs too? I know we have way too many of the already,
but if it doesn't need to be a typedef, it's better not to make it one.
> +static QLIST_HEAD(, IOHandlerRecord) io_handlers =
> +QLIST_HEAD_INITIALIZER(io_handlers);
> +
> +int vp_set_fd_handler2(int fd,
> + IOCanReadHandler *fd_read_poll,
> + IOHandler *fd_read,
> + IOHandler *fd_write,
> + void *opaque)
The formatting here is really odd, please make sure to align all
arguments, and maybe compress the lines a bit so it doesn't take up as
much realestate when you read the code.
> +int vp_send_all(int fd, const void *buf, int len1)
> +{
> +int ret, len;
> +
> +len = len1;
> +while (len > 0) {
> +ret = write(fd, buf, len);
> +if (ret < 0) {
> +if (errno != EINTR && errno != EAGAIN) {
> +warn("write() failed");
> +return -1;
Is -1 really an ideal error value to return here?
> +static void main_loop_wait(int nonblocking)
> +{
> +IOHandlerRecord *ioh;
> +fd_set rfds, wfds, xfds;
> +int ret, nfds;
> +struct timeval tv;
No good, please use qemu_timeval and friends for compatibility.
> +int timeout = 1000;
> +
> +if (nonblocking) {
> +timeout = 0;
> +}
> +
> +/* poll any events */
> +nfds = -1;
> +FD_ZERO(&rfds);
> +FD_ZERO(&wfds);
> +FD_ZERO(&xfds);
> +QLIST_FOREACH(ioh, &io_handlers, next) {
> +if (ioh->deleted)
> +continue;
Missing braces.
> +if (ioh->fd_read &&
> +(!ioh->fd_read_poll ||
> + ioh->fd_read_poll(ioh->opaque) != 0)) {
Put the || arguments on the same line so it is easier to read.
> +FD_SET(ioh->fd, &rfds);
> +if (ioh->fd > nfds)
> +nfds = ioh->fd;
Missing braces.
> +}
> +if (ioh->fd_write) {
> +FD_SET(ioh->fd, &wfds);
> +if (ioh->fd > nfds)
> +nfds = ioh->fd;
and again.
Cheers,
Jes
[Qemu-devel] [Bug 676029] Re: Silently fail with wrong vde socket dir
Hello,
Could you please provide more data, what kinda of system and version are
you running on?
Jes
--
Silently fail with wrong vde socket dir
https://bugs.launchpad.net/bugs/676029
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
Status in QEMU: New
Bug description:
Hi,
Using qemu 0.12.5, kvm silently fail with exit code 1 when using -net vde and a
wrong path for sock. Actually, the sock option is mean to be the socket dir of
the vde_switch, not the socket itself.
With -net vde,sock=/var/run/vde/vde0/ctl , strace ends with the following
messages :
connect(7, {sa_family=AF_FILE, path="/var/run/vde/vde0/ctl/ctl"}, 110) = -1
ENOTDIR (Not a directory)
close(7)= 0
close(8)= 0
exit_group(1) = ?
root ~#
Please add a meaningful message.
Regards,
Étienne
[Qemu-devel] Re: [PATCH v4] virtio-9p: fix build on !CONFIG_UTIMENSAT
On 11/18/10 09:48, Hidetoshi Seto wrote: > (2010/11/18 17:02), Jes Sorensen wrote: >> Hi Hidetoshi, >> >> I think the idea of the patch is good, but please move qemu_utimensat() >> to oslib-posix.c and provide a wrapper for oslib-win32.c. It is >> emulation for a system library function, so it doesn't belong in >> cutils.c, but rather in the oslib group. > > Unfortunately one fact is that I'm not familiar with win32 codes so I don't > have any idea how the wrapper for win32 will be... > If someone could kindly tell me about the win32 part, I could update this > patch to v5, but even though I have no test environment for the new part :-< > > Could we wait an incremental patch on this v4? > Can somebody help me? Volunteers? Hi Hidetoshi, I don't actually know much about win32 myself, the only thing I do is to try and cross-compile for it using mingw32 to make sure the build doesn't break. One option is to leave it open, or put in a dummy wrapper which asserts in the win32 part of the code, so that someone who is interested in win32 can fix it up. That should be pretty easy to do, and I think thats a fine starting point. Cheers, Jes
[Qemu-devel] Re: [PATCH v4] virtio-9p: fix build on !CONFIG_UTIMENSAT
On 11/18/10 01:41, Hidetoshi Seto wrote: > This patch introduce a fallback mechanism for old systems that do not > support utimensat(). This fix build failure with following warnings: > > hw/virtio-9p-local.c: In function 'local_utimensat': > hw/virtio-9p-local.c:479: warning: implicit declaration of function > 'utimensat' > hw/virtio-9p-local.c:479: warning: nested extern declaration of 'utimensat' > > and: > > hw/virtio-9p.c: In function 'v9fs_setattr_post_chmod': > hw/virtio-9p.c:1410: error: 'UTIME_NOW' undeclared (first use in this > function) > hw/virtio-9p.c:1410: error: (Each undeclared identifier is reported only once > hw/virtio-9p.c:1410: error: for each function it appears in.) > hw/virtio-9p.c:1413: error: 'UTIME_OMIT' undeclared (first use in this > function) > hw/virtio-9p.c: In function 'v9fs_wstat_post_chmod': > hw/virtio-9p.c:2905: error: 'UTIME_OMIT' undeclared (first use in this > function) > > v4: > - Use tv_now.tv_usec > - Rebased on latest qemu.git > v3: > - Use better alternative handling for UTIME_NOW/OMIT > - Move qemu_utimensat() to cutils.c > V2: > - Introduce qemu_utimensat() > > Acked-by: Chris Wright > Acked-by: M. Mohan Kumar > Signed-off-by: Hidetoshi Seto Hi Hidetoshi, I think the idea of the patch is good, but please move qemu_utimensat() to oslib-posix.c and provide a wrapper for oslib-win32.c. It is emulation for a system library function, so it doesn't belong in cutils.c, but rather in the oslib group. Thanks, Jes
Re: [Qemu-devel] Access to specific isa card with Qemu???
On 11/08/10 17:21, Djamel Hakkar wrote: > Hello, > > We have a software that runs on MS-DOS and must communicate with a specific > card installed on port isa. > We want to use this software in Qemu with a machine that runs XP. > Is it possible to access to the ISA port with Qemu in this case? > > Do we have to do a specific development? > Can you help us in this development, how much would it cost? Right now I don't think there is any support for ISA pass-through, but it could probably be done, even if it sounds pretty scary. There's a lot of risk with ISA cards, they can easily take down the whole system, and if it does DMA you are probably out of luck. You might find someone on the list willing to do the work on a contract, but I will let interested parties reply to you directly. I cannot do it. Cheers, Jes
Re: [Qemu-devel] [PATCH] virtio-9p: fix build on !CONFIG_UTIMENSAT v2
On 11/08/10 07:44, M. Mohan Kumar wrote:
>> This patch introduce a fallback mechanism for old systems that do not
>> support utimensat. This will fix build failure with following warnings:
>>
>> hw/virtio-9p-local.c: In function 'local_utimensat':
>> hw/virtio-9p-local.c:479: warning: implicit declaration of function
>> 'utimensat' hw/virtio-9p-local.c:479: warning: nested extern declaration
>> of 'utimensat'
>>
>> and
>>
>> hw/virtio-9p.c: In function 'v9fs_setattr_post_chmod':
>> hw/virtio-9p.c:1410: error: 'UTIME_NOW' undeclared (first use in this
>> function) hw/virtio-9p.c:1410: error: (Each undeclared identifier is
>> reported only once hw/virtio-9p.c:1410: error: for each function it
>> appears in.)
>> hw/virtio-9p.c:1413: error: 'UTIME_OMIT' undeclared (first use in this
>> function) hw/virtio-9p.c: In function 'v9fs_wstat_post_chmod':
>> hw/virtio-9p.c:2905: error: 'UTIME_OMIT' undeclared (first use in this
>> function)
>>
>> Signed-off-by: Hidetoshi Seto
>> ---
>> hw/virtio-9p-local.c | 32 ++--
>> hw/virtio-9p.h | 10 ++
>> 2 files changed, 40 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
>> index 0d52020..7811d2c 100644
>> --- a/hw/virtio-9p-local.c
>> +++ b/hw/virtio-9p-local.c
>> @@ -479,10 +479,38 @@ static int local_chown(FsContext *fs_ctx, const char
>> *path, FsCred *credp) return -1;
>> }
>>
>> +/* TODO: relocate this to proper file, and make it more generic */
>> +static int qemu_utimensat(int dirfd, const char *path,
>> + const struct timespec *times, int flags)
>> +{
>
> IMHO, this code can be moved to cutils.c
It's not a C utility function, so it really belongs in oslib-posix.c,
but otherwise I agree. This is emulation of a C library function, it
shouldn't be in the 9p code.
Cheers,
Jes
[Qemu-devel] [PATCH] Add missing braces
From: Jes Sorensen
This patch adds missing braces around if/else statements that call
macros which are likely to result in errors if the macro is
changed. It also makes the code comply better with CODING_STYLE.
Signed-off-by: Jes Sorensen
---
hw/e1000.c | 13 -
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/hw/e1000.c b/hw/e1000.c
index 532efdc..724dfbe 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -444,9 +444,10 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
// data descriptor
tp->sum_needed = le32_to_cpu(dp->upper.data) >> 8;
tp->cptse = ( txd_lower & E1000_TXD_CMD_TSE ) ? 1 : 0;
-} else
+} else {
// legacy descriptor
tp->cptse = 0;
+}
if (vlan_enabled(s) && is_vlan_txd(txd_lower) &&
(tp->cptse || txd_lower & E1000_TXD_CMD_EOP)) {
@@ -682,8 +683,9 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf,
size_t size)
(void *)(buf + vlan_offset), size);
desc.length = cpu_to_le16(size + fcs_len(s));
desc.status |= E1000_RXD_STAT_EOP|E1000_RXD_STAT_IXSM;
-} else // as per intel docs; skip descriptors with null buf addr
+} else { // as per intel docs; skip descriptors with null buf addr
DBGOUT(RX, "Null RX descriptor!!\n");
+}
cpu_physical_memory_write(base, (void *)&desc, sizeof(desc));
if (++s->mac_reg[RDH] * sizeof(desc) >= s->mac_reg[RDLEN])
@@ -855,13 +857,14 @@ e1000_mmio_writel(void *opaque, target_phys_addr_t addr,
uint32_t val)
#ifdef TARGET_WORDS_BIGENDIAN
val = bswap32(val);
#endif
-if (index < NWRITEOPS && macreg_writeops[index])
+if (index < NWRITEOPS && macreg_writeops[index]) {
macreg_writeops[index](s, index, val);
-else if (index < NREADOPS && macreg_readops[index])
+} else if (index < NREADOPS && macreg_readops[index]) {
DBGOUT(MMIO, "e1000_mmio_writel RO %x: 0x%04x\n", index<<2, val);
-else
+} else {
DBGOUT(UNKNOWN, "MMIO unknown write addr=0x%08x,val=0x%08x\n",
index<<2, val);
+}
}
static void
--
1.7.3.2
Re: [Qemu-devel] Re: [PATCH] pc: Fix e820 fw_cfg for big endian
On 11/09/10 13:42, Alexander Graf wrote: > > On 09.11.2010, at 11:57, Jes Sorensen wrote: > >> On 11/08/10 04:57, Alex Williamson wrote: >>> Signed-off-by: Alex Williamson >>> --- >>> >>> Compile tested only. Only current user is kvm, no cross-arch users. >>> >>> hw/pc.c | 14 +++--- >>> 1 files changed, 7 insertions(+), 7 deletions(-) >> >> Patch looks fine to me, but are there any systems out there using e820 >> on big endian hardware? > > This fixes things when host endianness is big endian. Ah right, then it's all fine :) Cheers, Jes
[Qemu-devel] Re: [PATCH] pc: Fix e820 fw_cfg for big endian
On 11/08/10 04:57, Alex Williamson wrote: > Signed-off-by: Alex Williamson > --- > > Compile tested only. Only current user is kvm, no cross-arch users. > > hw/pc.c | 14 +++--- > 1 files changed, 7 insertions(+), 7 deletions(-) Patch looks fine to me, but are there any systems out there using e820 on big endian hardware? Jes
[Qemu-devel] [PATCH 1/1] Fold send_all() wrapper unix_write() into one function
From: Jes Sorensen
The current send_all() wrapper for POSIX calls does nothing but call
unix_write(). Merge them to simplify the code.
Signed-off-by: Jes Sorensen
---
qemu-char.c |8 ++--
1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 6d2dce7..88997f9 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -508,9 +508,10 @@ int send_all(int fd, const void *buf, int len1)
#else
-static int unix_write(int fd, const uint8_t *buf, int len1)
+int send_all(int fd, const void *_buf, int len1)
{
int ret, len;
+const uint8_t *buf = _buf;
len = len1;
while (len > 0) {
@@ -527,11 +528,6 @@ static int unix_write(int fd, const uint8_t *buf, int len1)
}
return len1 - len;
}
-
-int send_all(int fd, const void *buf, int len1)
-{
-return unix_write(fd, buf, len1);
-}
#endif /* !_WIN32 */
#ifndef _WIN32
--
1.7.2.3
[Qemu-devel] [PATCH 9/9] Remove unncessary includes
From: Jes Sorensen No need to include stdlib.h for BSD as it is included by qemu-common.h, windows.h is handled by sysemu.h and osdep.c no longer needs malloc.h Signed-off-by: Jes Sorensen --- osdep.c |8 1 files changed, 0 insertions(+), 8 deletions(-) diff --git a/osdep.c b/osdep.c index 0d48561..327583b 100644 --- a/osdep.c +++ b/osdep.c @@ -44,14 +44,6 @@ extern int madvise(caddr_t, size_t, int); #endif -#ifdef _WIN32 -#include -#elif defined(CONFIG_BSD) -#include -#else -#include -#endif - #include "qemu-common.h" #include "trace.h" #include "sysemu.h" -- 1.7.2.3
[Qemu-devel] [PATCH 5/9] Move qemu_gettimeofday() to OS specific files
From: Jes Sorensen
Signed-off-by: Jes Sorensen
---
m68k-semi.c|2 +-
osdep.c| 31 ---
osdep.h| 15 ---
oslib-win32.c | 27 +++
posix-aio-compat.c |1 +
qemu-common.h |5 +
qemu-img.c |1 +
qemu-os-posix.h|3 +++
qemu-os-win32.h|8
qemu-tool.c|1 +
10 files changed, 47 insertions(+), 47 deletions(-)
diff --git a/m68k-semi.c b/m68k-semi.c
index d16bc67..0371089 100644
--- a/m68k-semi.c
+++ b/m68k-semi.c
@@ -33,10 +33,10 @@
#define SEMIHOSTING_HEAP_SIZE (128 * 1024 * 1024)
#else
#include "qemu-common.h"
-#include "sysemu.h"
#include "gdbstub.h"
#include "softmmu-semi.h"
#endif
+#include "sysemu.h"
#define HOSTED_EXIT 0
#define HOSTED_INIT_SIM 1
diff --git a/osdep.c b/osdep.c
index cb12e5f..b1664ac 100644
--- a/osdep.c
+++ b/osdep.c
@@ -111,37 +111,6 @@ int qemu_create_pidfile(const char *filename)
return 0;
}
-#ifdef _WIN32
-
-/* mingw32 needs ffs for compilations without optimization. */
-int ffs(int i)
-{
-/* Use gcc's builtin ffs. */
-return __builtin_ffs(i);
-}
-
-/* Offset between 1/1/1601 and 1/1/1970 in 100 nanosec units */
-#define _W32_FT_OFFSET (1164447360ULL)
-
-int qemu_gettimeofday(qemu_timeval *tp)
-{
- union {
-unsigned long long ns100; /*time since 1 Jan 1601 in 100ns units */
-FILETIME ft;
- } _now;
-
- if(tp)
-{
- GetSystemTimeAsFileTime (&_now.ft);
- tp->tv_usec=(long)((_now.ns100 / 10ULL) % 100ULL );
- tp->tv_sec= (long)((_now.ns100 - _W32_FT_OFFSET) / 1000ULL);
-}
- /* Always return 0 as per Open Group Base Specifications Issue 6.
- Do not set errno on error. */
- return 0;
-}
-#endif /* _WIN32 */
-
/*
* Opens a file with FD_CLOEXEC set
diff --git a/osdep.h b/osdep.h
index 6716281..8bd30d7 100644
--- a/osdep.h
+++ b/osdep.h
@@ -127,19 +127,4 @@ int qemu_madvise(void *addr, size_t len, int advice);
int qemu_create_pidfile(const char *filename);
-#ifdef _WIN32
-int ffs(int i);
-
-int setenv(const char *name, const char *value, int overwrite);
-
-typedef struct {
-long tv_sec;
-long tv_usec;
-} qemu_timeval;
-int qemu_gettimeofday(qemu_timeval *tp);
-#else
-typedef struct timeval qemu_timeval;
-#define qemu_gettimeofday(tp) gettimeofday(tp, NULL);
-#endif /* !_WIN32 */
-
#endif
diff --git a/oslib-win32.c b/oslib-win32.c
index 1ddd857..e03c472 100644
--- a/oslib-win32.c
+++ b/oslib-win32.c
@@ -92,3 +92,30 @@ int inet_aton(const char *cp, struct in_addr *ia)
void qemu_set_cloexec(int fd)
{
}
+
+/* mingw32 needs ffs for compilations without optimization. */
+int ffs(int i)
+{
+/* Use gcc's builtin ffs. */
+return __builtin_ffs(i);
+}
+
+/* Offset between 1/1/1601 and 1/1/1970 in 100 nanosec units */
+#define _W32_FT_OFFSET (1164447360ULL)
+
+int qemu_gettimeofday(qemu_timeval *tp)
+{
+ union {
+unsigned long long ns100; /*time since 1 Jan 1601 in 100ns units */
+FILETIME ft;
+ } _now;
+
+ if(tp) {
+ GetSystemTimeAsFileTime (&_now.ft);
+ tp->tv_usec=(long)((_now.ns100 / 10ULL) % 100ULL );
+ tp->tv_sec= (long)((_now.ns100 - _W32_FT_OFFSET) / 1000ULL);
+ }
+ /* Always return 0 as per Open Group Base Specifications Issue 6.
+ Do not set errno on error. */
+ return 0;
+}
diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index 7b862b5..fa5494d 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -24,6 +24,7 @@
#include "qemu-queue.h"
#include "osdep.h"
+#include "sysemu.h"
#include "qemu-common.h"
#include "trace.h"
#include "block_int.h"
diff --git a/qemu-common.h b/qemu-common.h
index 2fbc27f..898da62 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -167,6 +167,11 @@ const char *path(const char *pathname);
#define qemu_isascii(c)isascii((unsigned char)(c))
#define qemu_toascii(c)toascii((unsigned char)(c))
+#ifdef _WIN32
+/* ffs() in oslib-win32.c for WIN32, strings.h for the rest of the world */
+int ffs(int i);
+#endif
+
void *qemu_malloc(size_t size);
void *qemu_realloc(void *ptr, size_t size);
void *qemu_mallocz(size_t size);
diff --git a/qemu-img.c b/qemu-img.c
index 578b8eb..5b2bed3 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -24,6 +24,7 @@
#include "qemu-common.h"
#include "qemu-option.h"
#include "osdep.h"
+#include "sysemu.h"
#include "block_int.h"
#include
diff --git a/qemu-os-posix.h b/qemu-os-posix.h
index ed5c058..353f878 100644
--- a/qemu-os-posix.h
+++ b/qemu-os-posix.h
@@ -36,4 +36,7 @@ void os_setup_signal_handling(void);
void os_daemonize(void);
void os_setup_post(void);
+typedef struct timeval qemu_timeval;
+#define qemu_gettimeofday(tp) gettimeofday(tp, NULL)
+
#endif
dif
[Qemu-devel] [PATCH 1/9] Move QEMU OS dependant library functions to OS specific files
From: Jes Sorensen
This moves library functions used by both QEMU and the QEMU tools,
such as qemu-img, qemu-nbd etc. from osdep.c to oslib-{posix,win32}.c
In addition it introduces oslib-obj.y to the Makefile set to be
included by the various targets, instead of relying on these library
functions magically getting included via block-obj-y.
Signed-off-by: Jes Sorensen
---
Makefile |6 ++--
Makefile.objs |9 +-
osdep.c | 85 -
oslib-posix.c | 74 +
oslib-win32.c | 73 +
5 files changed, 158 insertions(+), 89 deletions(-)
create mode 100644 oslib-posix.c
create mode 100644 oslib-win32.c
diff --git a/Makefile b/Makefile
index a1434b1..4121338 100644
--- a/Makefile
+++ b/Makefile
@@ -129,11 +129,11 @@ version-obj-$(CONFIG_WIN32) += version.o
qemu-img.o: qemu-img-cmds.h
qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o: $(GENERATED_HEADERS)
-qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(trace-obj-y)
$(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o
+qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(oslib-obj-y)
$(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y)
qemu-timer-common.o
-qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(trace-obj-y)
$(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o
+qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(oslib-obj-y)
$(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y)
qemu-timer-common.o
-qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(trace-obj-y)
$(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o
+qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(oslib-obj-y)
$(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y)
qemu-timer-common.o
qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@," GEN $@")
diff --git a/Makefile.objs b/Makefile.objs
index f07fb01..c88e82d 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -5,10 +5,16 @@ qobject-obj-y += qjson.o json-lexer.o json-streamer.o
json-parser.o
qobject-obj-y += qerror.o
###
+# oslib-obj-y is code depending on the OS (win32 vs posix)
+oslib-obj-y = osdep.o
+oslib-obj-$(CONFIG_WIN32) += oslib-win32.o
+oslib-obj-$(CONFIG_POSIX) += oslib-posix.o
+
+###
# block-obj-y is code used by both qemu system emulation and qemu-img
block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
-block-obj-y += nbd.o block.o aio.o aes.o osdep.o qemu-config.o
+block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o
block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
@@ -50,6 +56,7 @@ common-obj-y += $(net-obj-y)
common-obj-y += $(qobject-obj-y)
common-obj-$(CONFIG_LINUX) += $(fsdev-obj-$(CONFIG_LINUX))
common-obj-y += readline.o console.o cursor.o async.o qemu-error.o
+common-obj-y += $(oslib-obj-y)
common-obj-$(CONFIG_WIN32) += os-win32.o
common-obj-$(CONFIG_POSIX) += os-posix.o
diff --git a/osdep.c b/osdep.c
index 2e05b21..581768a 100644
--- a/osdep.c
+++ b/osdep.c
@@ -61,91 +61,6 @@ extern int madvise(caddr_t, size_t, int);
#include "sysemu.h"
#include "qemu_socket.h"
-#if !defined(_POSIX_C_SOURCE) || defined(_WIN32) || defined(__sun__)
-static void *oom_check(void *ptr)
-{
-if (ptr == NULL) {
-#if defined(_WIN32)
-fprintf(stderr, "Failed to allocate memory: %lu\n", GetLastError());
-#else
-fprintf(stderr, "Failed to allocate memory: %s\n", strerror(errno));
-#endif
-abort();
-}
-return ptr;
-}
-#endif
-
-#if defined(_WIN32)
-void *qemu_memalign(size_t alignment, size_t size)
-{
-void *ptr;
-
-if (!size) {
-abort();
-}
-ptr = oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
-trace_qemu_memalign(alignment, size, ptr);
-return ptr;
-}
-
-void *qemu_vmalloc(size_t size)
-{
-void *ptr;
-
-/* FIXME: this is not exactly optimal solution since VirtualAlloc
- has 64Kb granularity, but at least it guarantees us that the
- memory is page aligned. */
-if (!size) {
-abort();
-}
-ptr = oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
-trace_qemu_vmalloc(size, ptr);
-return ptr;
-}
-
-void qemu_vfree(void *ptr)
-{
-trace_qemu_vfree(ptr);
-VirtualFree(ptr, 0, MEM_RELEASE);
-}
-
-#else
-
-void *qemu_memalign(size_t alignment, size_t size)
-{
-void *ptr;
-#if defined(_POSIX_C_SOURCE) && !defined(__sun__)
-int ret;
-ret = posix_memalign(&ptr, alignment, size);
-if (ret != 0) {
-fprintf(stderr
[Qemu-devel] [PATCH 7/9] Separate qemu_pidfile() into OS specific versions
From: Jes Sorensen
Signed-off-by: Jes Sorensen
---
os-posix.c | 21 +
os-win32.c | 24
osdep.c| 38 --
3 files changed, 45 insertions(+), 38 deletions(-)
diff --git a/os-posix.c b/os-posix.c
index 612b641..38c29d1 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -361,3 +361,24 @@ int qemu_eventfd(int fds[2])
return qemu_pipe(fds);
}
+
+int qemu_create_pidfile(const char *filename)
+{
+char buffer[128];
+int len;
+int fd;
+
+fd = qemu_open(filename, O_RDWR | O_CREAT, 0600);
+if (fd == -1) {
+return -1;
+}
+if (lockf(fd, F_TLOCK, 0) == -1) {
+return -1;
+}
+len = snprintf(buffer, sizeof(buffer), "%ld\n", (long)getpid());
+if (write(fd, buffer, len) != len) {
+return -1;
+}
+
+return 0;
+}
diff --git a/os-win32.c b/os-win32.c
index 3c6f50f..566d5e9 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -240,3 +240,27 @@ void os_pidfile_error(void)
{
fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno));
}
+
+int qemu_create_pidfile(const char *filename)
+{
+char buffer[128];
+int len;
+HANDLE file;
+OVERLAPPED overlap;
+BOOL ret;
+memset(&overlap, 0, sizeof(overlap));
+
+file = CreateFile(filename, GENERIC_WRITE, FILE_SHARE_READ, NULL,
+ OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
+
+if (file == INVALID_HANDLE_VALUE) {
+return -1;
+}
+len = snprintf(buffer, sizeof(buffer), "%ld\n", (long)getpid());
+ret = WriteFileEx(file, (LPCVOID)buffer, (DWORD)len,
+ &overlap, NULL);
+if (ret == 0) {
+return -1;
+}
+return 0;
+}
diff --git a/osdep.c b/osdep.c
index b1664ac..0d48561 100644
--- a/osdep.c
+++ b/osdep.c
@@ -73,44 +73,6 @@ int qemu_madvise(void *addr, size_t len, int advice)
#endif
}
-int qemu_create_pidfile(const char *filename)
-{
-char buffer[128];
-int len;
-#ifndef _WIN32
-int fd;
-
-fd = qemu_open(filename, O_RDWR | O_CREAT, 0600);
-if (fd == -1)
-return -1;
-
-if (lockf(fd, F_TLOCK, 0) == -1)
-return -1;
-
-len = snprintf(buffer, sizeof(buffer), "%ld\n", (long)getpid());
-if (write(fd, buffer, len) != len)
-return -1;
-#else
-HANDLE file;
-OVERLAPPED overlap;
-BOOL ret;
-memset(&overlap, 0, sizeof(overlap));
-
-file = CreateFile(filename, GENERIC_WRITE, FILE_SHARE_READ, NULL,
- OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
-
-if (file == INVALID_HANDLE_VALUE)
- return -1;
-
-len = snprintf(buffer, sizeof(buffer), "%ld\n", (long)getpid());
-ret = WriteFileEx(file, (LPCVOID)buffer, (DWORD)len,
- &overlap, NULL);
-if (ret == 0)
- return -1;
-#endif
-return 0;
-}
-
/*
* Opens a file with FD_CLOEXEC set
--
1.7.2.3
[Qemu-devel] [PATCH 8/9] Consolidate oom_check() functions
From: Jes Sorensen
This consolidates the duplicated oom_check() functions, as well as
splitting them into OS dependant versions to avoid the #ifdef
grossness that was present in the old osdep.c version.
Signed-off-by: Jes Sorensen
---
Makefile.target |2 +-
oslib-posix.c |8 +++-
oslib-win32.c |6 +++---
qemu-common.h |1 +
qemu-malloc.c | 14 +++---
5 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/Makefile.target b/Makefile.target
index c48cbcc..91e6e74 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -88,7 +88,7 @@ $(call set-vpath,
$(SRC_PATH)/linux-user:$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR
QEMU_CFLAGS+=-I$(SRC_PATH)/linux-user
-I$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR)
obj-y = main.o syscall.o strace.o mmap.o signal.o thunk.o \
elfload.o linuxload.o uaccess.o gdbstub.o cpu-uname.o \
- qemu-malloc.o
+ qemu-malloc.o $(oslib-obj-y)
obj-$(TARGET_HAS_BFLT) += flatload.o
diff --git a/oslib-posix.c b/oslib-posix.c
index ad44b17..6e9b0c3 100644
--- a/oslib-posix.c
+++ b/oslib-posix.c
@@ -31,8 +31,7 @@
#include "trace.h"
#include "qemu_socket.h"
-#if !defined(_POSIX_C_SOURCE) || defined(__sun__)
-static void *oom_check(void *ptr)
+void *qemu_oom_check(void *ptr)
{
if (ptr == NULL) {
fprintf(stderr, "Failed to allocate memory: %s\n", strerror(errno));
@@ -40,7 +39,6 @@ static void *oom_check(void *ptr)
}
return ptr;
}
-#endif
void *qemu_memalign(size_t alignment, size_t size)
{
@@ -54,9 +52,9 @@ void *qemu_memalign(size_t alignment, size_t size)
abort();
}
#elif defined(CONFIG_BSD)
-ptr = oom_check(valloc(size));
+ptr = qemu_oom_check(valloc(size));
#else
-ptr = oom_check(memalign(alignment, size));
+ptr = qemu_oom_check(memalign(alignment, size));
#endif
trace_qemu_memalign(alignment, size, ptr);
return ptr;
diff --git a/oslib-win32.c b/oslib-win32.c
index e03c472..ab29eae 100644
--- a/oslib-win32.c
+++ b/oslib-win32.c
@@ -31,7 +31,7 @@
#include "trace.h"
#include "qemu_socket.h"
-static void *oom_check(void *ptr)
+void *qemu_oom_check(void *ptr)
{
if (ptr == NULL) {
fprintf(stderr, "Failed to allocate memory: %lu\n", GetLastError());
@@ -47,7 +47,7 @@ void *qemu_memalign(size_t alignment, size_t size)
if (!size) {
abort();
}
-ptr = oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
+ptr = qemu_oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
trace_qemu_memalign(alignment, size, ptr);
return ptr;
}
@@ -62,7 +62,7 @@ void *qemu_vmalloc(size_t size)
if (!size) {
abort();
}
-ptr = oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
+ptr = qemu_oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
trace_qemu_vmalloc(size, ptr);
return ptr;
}
diff --git a/qemu-common.h b/qemu-common.h
index 898da62..12fb42d 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -172,6 +172,7 @@ const char *path(const char *pathname);
int ffs(int i);
#endif
+void *qemu_oom_check(void *ptr);
void *qemu_malloc(size_t size);
void *qemu_realloc(void *ptr, size_t size);
void *qemu_mallocz(size_t size);
diff --git a/qemu-malloc.c b/qemu-malloc.c
index ecffb67..28fb05a 100644
--- a/qemu-malloc.c
+++ b/qemu-malloc.c
@@ -25,14 +25,6 @@
#include "trace.h"
#include
-static void *oom_check(void *ptr)
-{
-if (ptr == NULL) {
-abort();
-}
-return ptr;
-}
-
void qemu_free(void *ptr)
{
trace_qemu_free(ptr);
@@ -54,7 +46,7 @@ void *qemu_malloc(size_t size)
if (!size && !allow_zero_malloc()) {
abort();
}
-ptr = oom_check(malloc(size ? size : 1));
+ptr = qemu_oom_check(malloc(size ? size : 1));
trace_qemu_malloc(size, ptr);
return ptr;
}
@@ -65,7 +57,7 @@ void *qemu_realloc(void *ptr, size_t size)
if (!size && !allow_zero_malloc()) {
abort();
}
-newptr = oom_check(realloc(ptr, size ? size : 1));
+newptr = qemu_oom_check(realloc(ptr, size ? size : 1));
trace_qemu_realloc(ptr, size, newptr);
return newptr;
}
@@ -75,7 +67,7 @@ void *qemu_mallocz(size_t size)
if (!size && !allow_zero_malloc()) {
abort();
}
-return oom_check(calloc(1, size ? size : 1));
+return qemu_oom_check(calloc(1, size ? size : 1));
}
char *qemu_strdup(const char *str)
--
1.7.2.3
[Qemu-devel] [PATCH 6/9] Do not redefine reserved key-words TRUE/FALSE
From: Jes Sorensen
TRUE/FALSE are generally reserved keywords and shouldn't be defined in
a driver like this. Rename the macros to SDP_TRUE and SDP_FALSE
respectively.
Signed-off-by: Jes Sorensen
---
hw/bt-sdp.c | 20 ++--
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/hw/bt-sdp.c b/hw/bt-sdp.c
index cc0bf2f..cdf2d95 100644
--- a/hw/bt-sdp.c
+++ b/hw/bt-sdp.c
@@ -786,11 +786,11 @@ static void sdp_service_db_build(struct
bt_l2cap_sdp_state_s *sdp,
.type = SDP_DTYPE_UUID | SDP_DSIZE_16, \
.value.uint = val, \
},
-#define TRUE { \
+#define SDP_TRUE { \
.type = SDP_DTYPE_BOOL | SDP_DSIZE_1,\
.value.uint = 1, \
},
-#define FALSE { \
+#define SDP_FALSE { \
.type = SDP_DTYPE_BOOL | SDP_DSIZE_1,\
.value.uint = 0, \
},
@@ -842,8 +842,8 @@ SERVICE(hid,
/* TODO: extract from l2cap_device->device.class[0] */
ATTRIBUTE(DEVICE_SUBCLASS, UINT8(0x40))
ATTRIBUTE(COUNTRY_CODE,UINT8(0x15))
-ATTRIBUTE(VIRTUAL_CABLE, TRUE)
-ATTRIBUTE(RECONNECT_INITIATE, FALSE)
+ATTRIBUTE(VIRTUAL_CABLE, SDP_TRUE)
+ATTRIBUTE(RECONNECT_INITIATE, SDP_FALSE)
/* TODO: extract from hid->usbdev->report_desc */
ATTRIBUTE(DESCRIPTOR_LIST, LIST(
LIST(UINT8(0x22) ARRAY(
@@ -883,12 +883,12 @@ SERVICE(hid,
ATTRIBUTE(LANG_ID_BASE_LIST, LIST(
LIST(UINT16(0x0409) UINT16(0x0100))
))
-ATTRIBUTE(SDP_DISABLE, FALSE)
-ATTRIBUTE(BATTERY_POWER, TRUE)
-ATTRIBUTE(REMOTE_WAKEUP, TRUE)
-ATTRIBUTE(BOOT_DEVICE, TRUE) /* XXX: untested */
+ATTRIBUTE(SDP_DISABLE, SDP_FALSE)
+ATTRIBUTE(BATTERY_POWER, SDP_TRUE)
+ATTRIBUTE(REMOTE_WAKEUP, SDP_TRUE)
+ATTRIBUTE(BOOT_DEVICE, SDP_TRUE) /* XXX: untested */
ATTRIBUTE(SUPERVISION_TIMEOUT, UINT16(0x0c80))
-ATTRIBUTE(NORMALLY_CONNECTABLE,TRUE)
+ATTRIBUTE(NORMALLY_CONNECTABLE,SDP_TRUE)
ATTRIBUTE(PROFILE_VERSION, UINT16(0x0100))
)
@@ -936,7 +936,7 @@ SERVICE(pnp,
/* Profile specific */
ATTRIBUTE(SPECIFICATION_ID, UINT16(0x0100))
ATTRIBUTE(VERSION, UINT16(0x0100))
-ATTRIBUTE(PRIMARY_RECORD, TRUE)
+ATTRIBUTE(PRIMARY_RECORD, SDP_TRUE)
)
static int bt_l2cap_sdp_new_ch(struct bt_l2cap_device_s *dev,
--
1.7.2.3
[Qemu-devel] [PATCH 2/9] Move osdep socket code to oslib-{posix, win32}.c
From: Jes Sorensen
Signed-off-by: Jes Sorensen
---
osdep.c | 38 --
oslib-posix.c | 15 +++
oslib-win32.c | 21 +
3 files changed, 36 insertions(+), 38 deletions(-)
diff --git a/osdep.c b/osdep.c
index 581768a..902fce9 100644
--- a/osdep.c
+++ b/osdep.c
@@ -147,44 +147,6 @@ int qemu_gettimeofday(qemu_timeval *tp)
#endif /* _WIN32 */
-#ifdef _WIN32
-void socket_set_nonblock(int fd)
-{
-unsigned long opt = 1;
-ioctlsocket(fd, FIONBIO, &opt);
-}
-
-int inet_aton(const char *cp, struct in_addr *ia)
-{
-uint32_t addr = inet_addr(cp);
-if (addr == 0x)
- return 0;
-ia->s_addr = addr;
-return 1;
-}
-
-void qemu_set_cloexec(int fd)
-{
-}
-
-#else
-
-void socket_set_nonblock(int fd)
-{
-int f;
-f = fcntl(fd, F_GETFL);
-fcntl(fd, F_SETFL, f | O_NONBLOCK);
-}
-
-void qemu_set_cloexec(int fd)
-{
-int f;
-f = fcntl(fd, F_GETFD);
-fcntl(fd, F_SETFD, f | FD_CLOEXEC);
-}
-
-#endif
-
/*
* Opens a file with FD_CLOEXEC set
*/
diff --git a/oslib-posix.c b/oslib-posix.c
index df97304..aebe3ac 100644
--- a/oslib-posix.c
+++ b/oslib-posix.c
@@ -29,6 +29,7 @@
#include "config-host.h"
#include "sysemu.h"
#include "trace.h"
+#include "qemu_socket.h"
#if !defined(_POSIX_C_SOURCE) || defined(__sun__)
static void *oom_check(void *ptr)
@@ -72,3 +73,17 @@ void qemu_vfree(void *ptr)
trace_qemu_vfree(ptr);
free(ptr);
}
+
+void socket_set_nonblock(int fd)
+{
+int f;
+f = fcntl(fd, F_GETFL);
+fcntl(fd, F_SETFL, f | O_NONBLOCK);
+}
+
+void qemu_set_cloexec(int fd)
+{
+int f;
+f = fcntl(fd, F_GETFD);
+fcntl(fd, F_SETFD, f | FD_CLOEXEC);
+}
diff --git a/oslib-win32.c b/oslib-win32.c
index 3b5245d..1ddd857 100644
--- a/oslib-win32.c
+++ b/oslib-win32.c
@@ -29,6 +29,7 @@
#include "config-host.h"
#include "sysemu.h"
#include "trace.h"
+#include "qemu_socket.h"
static void *oom_check(void *ptr)
{
@@ -71,3 +72,23 @@ void qemu_vfree(void *ptr)
trace_qemu_vfree(ptr);
VirtualFree(ptr, 0, MEM_RELEASE);
}
+
+void socket_set_nonblock(int fd)
+{
+unsigned long opt = 1;
+ioctlsocket(fd, FIONBIO, &opt);
+}
+
+int inet_aton(const char *cp, struct in_addr *ia)
+{
+uint32_t addr = inet_addr(cp);
+if (addr == 0x) {
+ return 0;
+}
+ia->s_addr = addr;
+return 1;
+}
+
+void qemu_set_cloexec(int fd)
+{
+}
--
1.7.2.3
[Qemu-devel] [PATCH 4/9] We only support eventfd under POSIX, move qemu_eventfd() to os-posix.c
From: Jes Sorensen
Signed-off-by: Jes Sorensen
---
os-posix.c | 32
osdep.c| 34 --
2 files changed, 32 insertions(+), 34 deletions(-)
diff --git a/os-posix.c b/os-posix.c
index 6321e99..612b641 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -43,6 +43,10 @@
#include
#endif
+#ifdef CONFIG_EVENTFD
+#include
+#endif
+
static struct passwd *user_pwd;
static const char *chroot_dir;
static int daemonize;
@@ -329,3 +333,31 @@ void os_set_line_buffering(void)
{
setvbuf(stdout, NULL, _IOLBF, 0);
}
+
+/*
+ * Creates an eventfd that looks like a pipe and has EFD_CLOEXEC set.
+ */
+int qemu_eventfd(int fds[2])
+{
+#ifdef CONFIG_EVENTFD
+int ret;
+
+ret = eventfd(0, 0);
+if (ret >= 0) {
+fds[0] = ret;
+qemu_set_cloexec(ret);
+if ((fds[1] = dup(ret)) == -1) {
+close(ret);
+return -1;
+}
+qemu_set_cloexec(fds[1]);
+return 0;
+}
+
+if (errno != ENOSYS) {
+return -1;
+}
+#endif
+
+return qemu_pipe(fds);
+}
diff --git a/osdep.c b/osdep.c
index 926c8ad..cb12e5f 100644
--- a/osdep.c
+++ b/osdep.c
@@ -44,10 +44,6 @@
extern int madvise(caddr_t, size_t, int);
#endif
-#ifdef CONFIG_EVENTFD
-#include
-#endif
-
#ifdef _WIN32
#include
#elif defined(CONFIG_BSD)
@@ -207,36 +203,6 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t
count)
return total;
}
-#ifndef _WIN32
-/*
- * Creates an eventfd that looks like a pipe and has EFD_CLOEXEC set.
- */
-int qemu_eventfd(int fds[2])
-{
-#ifdef CONFIG_EVENTFD
-int ret;
-
-ret = eventfd(0, 0);
-if (ret >= 0) {
-fds[0] = ret;
-qemu_set_cloexec(ret);
-if ((fds[1] = dup(ret)) == -1) {
-close(ret);
-return -1;
-}
-qemu_set_cloexec(fds[1]);
-return 0;
-}
-
-if (errno != ENOSYS) {
-return -1;
-}
-#endif
-
-return qemu_pipe(fds);
-}
-#endif
-
/*
* Opens a socket with FD_CLOEXEC set
*/
--
1.7.2.3
[Qemu-devel] [PATCH 3/9] qemu_pipe() is used only by POSIX code, so move to oslib-posix.c
From: Jes Sorensen
Signed-off-by: Jes Sorensen
---
osdep.c | 22 --
oslib-posix.c | 22 ++
2 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/osdep.c b/osdep.c
index 902fce9..926c8ad 100644
--- a/osdep.c
+++ b/osdep.c
@@ -235,28 +235,6 @@ int qemu_eventfd(int fds[2])
return qemu_pipe(fds);
}
-
-/*
- * Creates a pipe with FD_CLOEXEC set on both file descriptors
- */
-int qemu_pipe(int pipefd[2])
-{
-int ret;
-
-#ifdef CONFIG_PIPE2
-ret = pipe2(pipefd, O_CLOEXEC);
-if (ret != -1 || errno != ENOSYS) {
-return ret;
-}
-#endif
-ret = pipe(pipefd);
-if (ret == 0) {
-qemu_set_cloexec(pipefd[0]);
-qemu_set_cloexec(pipefd[1]);
-}
-
-return ret;
-}
#endif
/*
diff --git a/oslib-posix.c b/oslib-posix.c
index aebe3ac..ad44b17 100644
--- a/oslib-posix.c
+++ b/oslib-posix.c
@@ -87,3 +87,25 @@ void qemu_set_cloexec(int fd)
f = fcntl(fd, F_GETFD);
fcntl(fd, F_SETFD, f | FD_CLOEXEC);
}
+
+/*
+ * Creates a pipe with FD_CLOEXEC set on both file descriptors
+ */
+int qemu_pipe(int pipefd[2])
+{
+int ret;
+
+#ifdef CONFIG_PIPE2
+ret = pipe2(pipefd, O_CLOEXEC);
+if (ret != -1 || errno != ENOSYS) {
+return ret;
+}
+#endif
+ret = pipe(pipefd);
+if (ret == 0) {
+qemu_set_cloexec(pipefd[0]);
+qemu_set_cloexec(pipefd[1]);
+}
+
+return ret;
+}
--
1.7.2.3
[Qemu-devel] [PATCH v4 0/9] Re-factor osdep code + macro and brace fixes
From: Jes Sorensen
Hi,
Here is another set of patches which tries to split up osdep.c further
into posix and win32 versions. It introduces oslib-{posix,win32}.c
files which are used for functions that are OS specific core library
functionality, like gettimeofday(), and which is used by both QEMU and
support applications like qemu-img. Other functions are moved to
os-{posix,win32}.c. In addtion there are a couple of minor fixes for
bad macro names.
In some cases braces were added to code when it was moved, to make it
compliant with the QEMU bracing rules.
v4 fixes the build problem for m68k-linux-user reported by Blue Swirl.
Cheers,
Jes
Jes Sorensen (9):
Move QEMU OS dependant library functions to OS specific files
Move osdep socket code to oslib-{posix,win32}.c
qemu_pipe() is used only by POSIX code, so move to oslib-posix.c
We only support eventfd under POSIX, move qemu_eventfd() to
os-posix.c
Move qemu_gettimeofday() to OS specific files
Do not redefine reserved key-words TRUE/FALSE
Separate qemu_pidfile() into OS specific versions
Consolidate oom_check() functions
Remove unncessary includes
Makefile |6 +-
Makefile.objs |9 ++-
Makefile.target|2 +-
hw/bt-sdp.c| 20 ++--
m68k-semi.c|2 +-
os-posix.c | 53 +++
os-win32.c | 24 +
osdep.c| 256
osdep.h| 15 ---
oslib-posix.c | 109 ++
oslib-win32.c | 121 +
posix-aio-compat.c |1 +
qemu-common.h |6 ++
qemu-img.c |1 +
qemu-malloc.c | 14 +---
qemu-os-posix.h|3 +
qemu-os-win32.h|8 ++
qemu-tool.c|1 +
18 files changed, 353 insertions(+), 298 deletions(-)
create mode 100644 oslib-posix.c
create mode 100644 oslib-win32.c
--
1.7.2.3
[Qemu-devel] Re: [PATCH 5/9] Move qemu_gettimeofday() to OS specific files
On 10/23/10 16:42, Blue Swirl wrote: > On Mon, Oct 18, 2010 at 8:15 AM, wrote: >> From: Jes Sorensen >> >> Signed-off-by: Jes Sorensen > > Almost there: > CCm68k-linux-user/m68k-semi.o > /src/qemu/m68k-semi.c: In function 'do_m68k_semihosting': > /src/qemu/m68k-semi.c:328: error: 'qemu_timeval' undeclared (first use > in this function) > /src/qemu/m68k-semi.c:328: error: (Each undeclared identifier is > reported only once > /src/qemu/m68k-semi.c:328: error: for each function it appears in.) > /src/qemu/m68k-semi.c:328: error: expected ';' before 'tv' > cc1: warnings being treated as errors > /src/qemu/m68k-semi.c:330: error: implicit declaration of function > 'qemu_gettimeofday' > /src/qemu/m68k-semi.c:330: error: nested extern declaration of > 'qemu_gettimeofday' > /src/qemu/m68k-semi.c:330: error: 'tv' undeclared (first use in this function) How are you configuring your build? m68k-semi.c builds fine for me here with these patches applied: CCm68k-softmmu/mcf_fec.o CCm68k-softmmu/m68k-semi.o CCm68k-softmmu/dummy_m68k.o LINK m68k-softmmu/qemu-system-m68k This is for target m68k-softmmu, are you using a different target? Thanks, Jes
Re: [Qemu-devel] [PATCH v9 0/4] Introduce strtosz and make use of it
On 10/22/10 09:59, Markus Armbruster wrote: > jes.soren...@redhat.com writes: >> From: Jes Sorensen >> This patch introduces cutils.c: strtosz() and gets rid of the >> multiple custom hacks for parsing byte sizes. In addition it adds >> supports for specifying human style sizes such as 1.5G. Last it >> eliminates the horrible abuse of a float to store the byte size for >> migrate_set_speed in the monitor. >> >> Note, this is tested on Linux and build tested for win32 using >> mingw32. >> >> v9: I worked through a couple of revisions directly with Markus and I >> think I got it right finally. > > I'd prefer to have strtosz() to match strtol() & friends and not > restrict suffixes. But this code does what it claims to do, as far as I > can see, so: > > ACK series Thanks! I thought about this a fair bit and I believe doing the full test in the function is the most valuable. It's a personal preference obviously. I think it's a win to do it here since it simplifies the caller code. Would be great to get this applied so I can get it off my plate :) Cheers, Jes
[Qemu-devel] [PATCH 3/4] Switch migrate_set_speed() to take an 'o' argument rather than a float.
From: Jes Sorensen
Clarify default value of MB in migration speed argument in monitor, if
no suffix is specified. This differ from previous default of bytes,
but is consistent with the rest of the places where we accept a size
argument.
Signed-off-by: Jes Sorensen
---
hmp-commands.hx |5 +++--
migration.c |4 ++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 81999aa..e5585ba 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -754,9 +754,10 @@ ETEXI
{
.name = "migrate_set_speed",
-.args_type = "value:f",
+.args_type = "value:o",
.params = "value",
-.help = "set maximum speed (in bytes) for migrations",
+.help = "set maximum speed (in bytes) for migrations. "
+ "Defaults to MB if no size suffix is specified, ie. B/K/M/G/T",
.user_print = monitor_user_noop,
.mhandler.cmd_new = do_migrate_set_speed,
},
diff --git a/migration.c b/migration.c
index 468d517..9ee8b17 100644
--- a/migration.c
+++ b/migration.c
@@ -132,10 +132,10 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict,
QObject **ret_data)
int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
-double d;
+int64_t d;
FdMigrationState *s;
-d = qdict_get_double(qdict, "value");
+d = qdict_get_int(qdict, "value");
d = MAX(0, MIN(UINT32_MAX, d));
max_throttle = d;
--
1.7.2.3
[Qemu-devel] [PATCH 2/4] Add support for 'o' octet (bytes) format as monitor parameter.
From: Jes Sorensen
Octet format relies on strtosz which supports K/k, M/m, G/g, T/t
suffixes and unit support for humans, like 1.3G
Signed-off-by: Jes Sorensen
---
monitor.c | 29 +
1 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/monitor.c b/monitor.c
index 260cc02..6ef96a2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -78,6 +78,11 @@
* 'l' target long (32 or 64 bit)
* 'M' just like 'l', except in user mode the value is
* multiplied by 2^20 (think Mebibyte)
+ * 'o' octets (aka bytes)
+ * user mode accepts an optional T, t, G, g, M, m, K, k
+ * suffix, which multiplies the value by 2^40 for
+ * suffixes T and t, 2^30 for suffixes G and g, 2^20 for
+ * M and m, 2^10 for K and k
* 'f' double
* user mode accepts an optional G, g, M, m, K, k suffix,
* which multiplies the value by 2^30 for suffixes G and
@@ -3703,6 +3708,29 @@ static const mon_cmd_t *monitor_parse_command(Monitor
*mon,
qdict_put(qdict, key, qint_from_int(val));
}
break;
+case 'o':
+{
+ssize_t val;
+char *end;
+
+while (qemu_isspace(*p)) {
+p++;
+}
+if (*typestr == '?') {
+typestr++;
+if (*p == '\0') {
+break;
+}
+}
+val = strtosz(p, &end);
+if (val < 0) {
+monitor_printf(mon, "invalid size\n");
+goto fail;
+}
+qdict_put(qdict, key, qint_from_int(val));
+p = end;
+}
+break;
case 'f':
case 'T':
{
@@ -4200,6 +4228,7 @@ static int check_client_args_type(const QDict
*client_args,
case 'i':
case 'l':
case 'M':
+case 'o':
if (qobject_type(client_arg) != QTYPE_QINT) {
qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
"int");
--
1.7.2.3
[Qemu-devel] [PATCH 1/4] Introduce strtosz() library function to convert a string to a byte count.
From: Jes Sorensen
strtosz() returns -1 on error. It now supports human unit formats in
eg. 1.0G, with better error handling.
The following suffixes are supported:
B/b = bytes
K/k = KB
M/m = MB
G/g = GB
T/t = TB
This patch changes -numa and -m input to use strtosz().
Signed-off-by: Jes Sorensen
---
cutils.c | 88 +
qemu-common.h |1 +
vl.c | 31 ++-
3 files changed, 99 insertions(+), 21 deletions(-)
diff --git a/cutils.c b/cutils.c
index 5883737..28089aa 100644
--- a/cutils.c
+++ b/cutils.c
@@ -23,6 +23,7 @@
*/
#include "qemu-common.h"
#include "host-utils.h"
+#include
void pstrcpy(char *buf, int buf_size, const char *str)
{
@@ -283,3 +284,90 @@ int fcntl_setfl(int fd, int flag)
}
#endif
+/*
+ * Convert string to bytes, allowing either B/b for bytes, K/k for KB,
+ * M/m for MB, G/g for GB or T/t for TB. Default without any postfix
+ * is MB. End pointer will be returned in *end, if not NULL. A valid
+ * value must be terminated by whitespace, ',' or '\0'. Return -1 on
+ * error.
+ */
+ssize_t strtosz(const char *nptr, char **end)
+{
+ssize_t retval = -1;
+char *endptr, c;
+int mul_required = 0;
+double val, mul, integral, fraction;
+
+errno = 0;
+val = strtod(nptr, &endptr);
+if (isnan(val) || endptr == nptr || errno != 0) {
+goto fail;
+}
+integral = modf(val, &fraction);
+if (integral != 0) {
+mul_required = 1;
+}
+/*
+ * Any whitespace character is fine for terminating the number,
+ * in addition we accept ',' to handle strings where the size is
+ * part of a multi token argument.
+ */
+c = *endptr;
+if (isspace(c) || c == '\0' || c == ',') {
+c = 0;
+}
+switch (c) {
+case 'B':
+case 'b':
+mul = 1;
+if (mul_required) {
+goto fail;
+}
+break;
+case 'K':
+case 'k':
+mul = 1 << 10;
+break;
+case 0:
+if (mul_required) {
+goto fail;
+}
+case 'M':
+case 'm':
+mul = 1ULL << 20;
+break;
+case 'G':
+case 'g':
+mul = 1ULL << 30;
+break;
+case 'T':
+case 't':
+mul = 1ULL << 40;
+break;
+default:
+goto fail;
+}
+/*
+ * If not terminated by whitespace, ',', or \0, increment endptr
+ * to point to next character, then check that we are terminated
+ * by an appropriate separating character, ie. whitespace, ',', or
+ * \0. If not, we are seeing trailing garbage, thus fail.
+ */
+if (c != 0) {
+endptr++;
+if (!isspace(*endptr) && *endptr != ',' && *endptr != 0) {
+goto fail;
+}
+}
+if ((val * mul >= ~(size_t)0) || val < 0) {
+goto fail;
+}
+retval = val * mul;
+
+fail:
+if (end) {
+*end = endptr;
+}
+
+return retval;
+}
diff --git a/qemu-common.h b/qemu-common.h
index 81aafa0..0a062d4 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -153,6 +153,7 @@ time_t mktimegm(struct tm *tm);
int qemu_fls(int i);
int qemu_fdatasync(int fd);
int fcntl_setfl(int fd, int flag);
+ssize_t strtosz(const char *nptr, char **end);
/* path.c */
void init_paths(const char *prefix);
diff --git a/vl.c b/vl.c
index df414ef..6043fa2 100644
--- a/vl.c
+++ b/vl.c
@@ -734,16 +734,13 @@ static void numa_add(const char *optarg)
if (get_param_value(option, 128, "mem", optarg) == 0) {
node_mem[nodenr] = 0;
} else {
-value = strtoull(option, &endptr, 0);
-switch (*endptr) {
-case 0: case 'M': case 'm':
-value <<= 20;
-break;
-case 'G': case 'g':
-value <<= 30;
-break;
+ssize_t sval;
+sval = strtosz(option, NULL);
+if (sval < 0) {
+fprintf(stderr, "qemu: invalid numa mem size: %s\n", optarg);
+exit(1);
}
-node_mem[nodenr] = value;
+node_mem[nodenr] = sval;
}
if (get_param_value(option, 128, "cpus", optarg) == 0) {
node_cpumask[nodenr] = 0;
@@ -2163,18 +2160,10 @@ int main(int argc, char **argv, char **envp)
exit(0);
break;
case QEMU_OPTION_m: {
-uint64_t value;
-char *ptr;
+ssize_t value;
-value = strtoul(optarg, &ptr, 10);
-switch (*ptr) {
-case 0: case 'M':
[Qemu-devel] [PATCH 4/4] Remove obsolete 'f' double parameter type
From: Jes Sorensen
'f' double is no longer used, and we should be using floating point
variables to store byte sizes. Remove it.
Signed-off-by: Jes Sorensen
---
monitor.c | 18 +-
1 files changed, 1 insertions(+), 17 deletions(-)
diff --git a/monitor.c b/monitor.c
index 6ef96a2..ae5355d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -83,10 +83,6 @@
* suffix, which multiplies the value by 2^40 for
* suffixes T and t, 2^30 for suffixes G and g, 2^20 for
* M and m, 2^10 for K and k
- * 'f' double
- * user mode accepts an optional G, g, M, m, K, k suffix,
- * which multiplies the value by 2^30 for suffixes G and
- * g, 2^20 for M and m, 2^10 for K and k
* 'T' double
* user mode accepts an optional ms, us, ns suffix,
* which divides the value by 1e3, 1e6, 1e9, respectively
@@ -3731,7 +3727,6 @@ static const mon_cmd_t *monitor_parse_command(Monitor
*mon,
p = end;
}
break;
-case 'f':
case 'T':
{
double val;
@@ -3747,17 +3742,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor
*mon,
if (get_double(mon, &val, &p) < 0) {
goto fail;
}
-if (c == 'f' && *p) {
-switch (*p) {
-case 'K': case 'k':
-val *= 1 << 10; p++; break;
-case 'M': case 'm':
-val *= 1 << 20; p++; break;
-case 'G': case 'g':
-val *= 1 << 30; p++; break;
-}
-}
-if (c == 'T' && p[0] && p[1] == 's') {
+if (p[0] && p[1] == 's') {
switch (*p) {
case 'm':
val /= 1e3; p += 2; break;
@@ -4235,7 +4220,6 @@ static int check_client_args_type(const QDict
*client_args,
return -1;
}
break;
-case 'f':
case 'T':
if (qobject_type(client_arg) != QTYPE_QINT &&
qobject_type(client_arg) != QTYPE_QFLOAT) {
--
1.7.2.3
[Qemu-devel] [PATCH v9 0/4] Introduce strtosz and make use of it
From: Jes Sorensen This patch introduces cutils.c: strtosz() and gets rid of the multiple custom hacks for parsing byte sizes. In addition it adds supports for specifying human style sizes such as 1.5G. Last it eliminates the horrible abuse of a float to store the byte size for migrate_set_speed in the monitor. Note, this is tested on Linux and build tested for win32 using mingw32. v9: I worked through a couple of revisions directly with Markus and I think I got it right finally. Cheers, Jes Jes Sorensen (4): Introduce strtosz() library function to convert a string to a byte count. Add support for 'o' octet (bytes) format as monitor parameter. Switch migrate_set_speed() to take an 'o' argument rather than a float. Remove obsolete 'f' double parameter type cutils.c| 88 +++ hmp-commands.hx |5 ++- migration.c |4 +- monitor.c | 47 +++-- qemu-common.h |1 + vl.c| 31 ++- 6 files changed, 134 insertions(+), 42 deletions(-) -- 1.7.2.3
[Qemu-devel] [PATCH 8/9] Consolidate oom_check() functions
From: Jes Sorensen
This consolidates the duplicated oom_check() functions, as well as
splitting them into OS dependant versions to avoid the #ifdef
grossness that was present in the old osdep.c version.
Signed-off-by: Jes Sorensen
---
Makefile.target |2 +-
oslib-posix.c |8 +++-
oslib-win32.c |6 +++---
qemu-common.h |1 +
qemu-malloc.c | 14 +++---
5 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/Makefile.target b/Makefile.target
index c48cbcc..91e6e74 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -88,7 +88,7 @@ $(call set-vpath,
$(SRC_PATH)/linux-user:$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR
QEMU_CFLAGS+=-I$(SRC_PATH)/linux-user
-I$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR)
obj-y = main.o syscall.o strace.o mmap.o signal.o thunk.o \
elfload.o linuxload.o uaccess.o gdbstub.o cpu-uname.o \
- qemu-malloc.o
+ qemu-malloc.o $(oslib-obj-y)
obj-$(TARGET_HAS_BFLT) += flatload.o
diff --git a/oslib-posix.c b/oslib-posix.c
index ad44b17..6e9b0c3 100644
--- a/oslib-posix.c
+++ b/oslib-posix.c
@@ -31,8 +31,7 @@
#include "trace.h"
#include "qemu_socket.h"
-#if !defined(_POSIX_C_SOURCE) || defined(__sun__)
-static void *oom_check(void *ptr)
+void *qemu_oom_check(void *ptr)
{
if (ptr == NULL) {
fprintf(stderr, "Failed to allocate memory: %s\n", strerror(errno));
@@ -40,7 +39,6 @@ static void *oom_check(void *ptr)
}
return ptr;
}
-#endif
void *qemu_memalign(size_t alignment, size_t size)
{
@@ -54,9 +52,9 @@ void *qemu_memalign(size_t alignment, size_t size)
abort();
}
#elif defined(CONFIG_BSD)
-ptr = oom_check(valloc(size));
+ptr = qemu_oom_check(valloc(size));
#else
-ptr = oom_check(memalign(alignment, size));
+ptr = qemu_oom_check(memalign(alignment, size));
#endif
trace_qemu_memalign(alignment, size, ptr);
return ptr;
diff --git a/oslib-win32.c b/oslib-win32.c
index e03c472..ab29eae 100644
--- a/oslib-win32.c
+++ b/oslib-win32.c
@@ -31,7 +31,7 @@
#include "trace.h"
#include "qemu_socket.h"
-static void *oom_check(void *ptr)
+void *qemu_oom_check(void *ptr)
{
if (ptr == NULL) {
fprintf(stderr, "Failed to allocate memory: %lu\n", GetLastError());
@@ -47,7 +47,7 @@ void *qemu_memalign(size_t alignment, size_t size)
if (!size) {
abort();
}
-ptr = oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
+ptr = qemu_oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
trace_qemu_memalign(alignment, size, ptr);
return ptr;
}
@@ -62,7 +62,7 @@ void *qemu_vmalloc(size_t size)
if (!size) {
abort();
}
-ptr = oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
+ptr = qemu_oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
trace_qemu_vmalloc(size, ptr);
return ptr;
}
diff --git a/qemu-common.h b/qemu-common.h
index 1f01a44..82fb59f 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -179,6 +179,7 @@ const char *path(const char *pathname);
int ffs(int i);
#endif
+void *qemu_oom_check(void *ptr);
void *qemu_malloc(size_t size);
void *qemu_realloc(void *ptr, size_t size);
void *qemu_mallocz(size_t size);
diff --git a/qemu-malloc.c b/qemu-malloc.c
index ecffb67..28fb05a 100644
--- a/qemu-malloc.c
+++ b/qemu-malloc.c
@@ -25,14 +25,6 @@
#include "trace.h"
#include
-static void *oom_check(void *ptr)
-{
-if (ptr == NULL) {
-abort();
-}
-return ptr;
-}
-
void qemu_free(void *ptr)
{
trace_qemu_free(ptr);
@@ -54,7 +46,7 @@ void *qemu_malloc(size_t size)
if (!size && !allow_zero_malloc()) {
abort();
}
-ptr = oom_check(malloc(size ? size : 1));
+ptr = qemu_oom_check(malloc(size ? size : 1));
trace_qemu_malloc(size, ptr);
return ptr;
}
@@ -65,7 +57,7 @@ void *qemu_realloc(void *ptr, size_t size)
if (!size && !allow_zero_malloc()) {
abort();
}
-newptr = oom_check(realloc(ptr, size ? size : 1));
+newptr = qemu_oom_check(realloc(ptr, size ? size : 1));
trace_qemu_realloc(ptr, size, newptr);
return newptr;
}
@@ -75,7 +67,7 @@ void *qemu_mallocz(size_t size)
if (!size && !allow_zero_malloc()) {
abort();
}
-return oom_check(calloc(1, size ? size : 1));
+return qemu_oom_check(calloc(1, size ? size : 1));
}
char *qemu_strdup(const char *str)
--
1.7.2.3
[Qemu-devel] [PATCH 5/9] Move qemu_gettimeofday() to OS specific files
From: Jes Sorensen
Signed-off-by: Jes Sorensen
---
osdep.c| 31 ---
osdep.h| 15 ---
oslib-win32.c | 27 +++
posix-aio-compat.c |1 +
qemu-common.h |5 +
qemu-img.c |1 +
qemu-os-posix.h|3 +++
qemu-os-win32.h|8
qemu-tool.c|1 +
9 files changed, 46 insertions(+), 46 deletions(-)
diff --git a/osdep.c b/osdep.c
index cb12e5f..b1664ac 100644
--- a/osdep.c
+++ b/osdep.c
@@ -111,37 +111,6 @@ int qemu_create_pidfile(const char *filename)
return 0;
}
-#ifdef _WIN32
-
-/* mingw32 needs ffs for compilations without optimization. */
-int ffs(int i)
-{
-/* Use gcc's builtin ffs. */
-return __builtin_ffs(i);
-}
-
-/* Offset between 1/1/1601 and 1/1/1970 in 100 nanosec units */
-#define _W32_FT_OFFSET (1164447360ULL)
-
-int qemu_gettimeofday(qemu_timeval *tp)
-{
- union {
-unsigned long long ns100; /*time since 1 Jan 1601 in 100ns units */
-FILETIME ft;
- } _now;
-
- if(tp)
-{
- GetSystemTimeAsFileTime (&_now.ft);
- tp->tv_usec=(long)((_now.ns100 / 10ULL) % 100ULL );
- tp->tv_sec= (long)((_now.ns100 - _W32_FT_OFFSET) / 1000ULL);
-}
- /* Always return 0 as per Open Group Base Specifications Issue 6.
- Do not set errno on error. */
- return 0;
-}
-#endif /* _WIN32 */
-
/*
* Opens a file with FD_CLOEXEC set
diff --git a/osdep.h b/osdep.h
index 6716281..8bd30d7 100644
--- a/osdep.h
+++ b/osdep.h
@@ -127,19 +127,4 @@ int qemu_madvise(void *addr, size_t len, int advice);
int qemu_create_pidfile(const char *filename);
-#ifdef _WIN32
-int ffs(int i);
-
-int setenv(const char *name, const char *value, int overwrite);
-
-typedef struct {
-long tv_sec;
-long tv_usec;
-} qemu_timeval;
-int qemu_gettimeofday(qemu_timeval *tp);
-#else
-typedef struct timeval qemu_timeval;
-#define qemu_gettimeofday(tp) gettimeofday(tp, NULL);
-#endif /* !_WIN32 */
-
#endif
diff --git a/oslib-win32.c b/oslib-win32.c
index 1ddd857..e03c472 100644
--- a/oslib-win32.c
+++ b/oslib-win32.c
@@ -92,3 +92,30 @@ int inet_aton(const char *cp, struct in_addr *ia)
void qemu_set_cloexec(int fd)
{
}
+
+/* mingw32 needs ffs for compilations without optimization. */
+int ffs(int i)
+{
+/* Use gcc's builtin ffs. */
+return __builtin_ffs(i);
+}
+
+/* Offset between 1/1/1601 and 1/1/1970 in 100 nanosec units */
+#define _W32_FT_OFFSET (1164447360ULL)
+
+int qemu_gettimeofday(qemu_timeval *tp)
+{
+ union {
+unsigned long long ns100; /*time since 1 Jan 1601 in 100ns units */
+FILETIME ft;
+ } _now;
+
+ if(tp) {
+ GetSystemTimeAsFileTime (&_now.ft);
+ tp->tv_usec=(long)((_now.ns100 / 10ULL) % 100ULL );
+ tp->tv_sec= (long)((_now.ns100 - _W32_FT_OFFSET) / 1000ULL);
+ }
+ /* Always return 0 as per Open Group Base Specifications Issue 6.
+ Do not set errno on error. */
+ return 0;
+}
diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index 7b862b5..fa5494d 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -24,6 +24,7 @@
#include "qemu-queue.h"
#include "osdep.h"
+#include "sysemu.h"
#include "qemu-common.h"
#include "trace.h"
#include "block_int.h"
diff --git a/qemu-common.h b/qemu-common.h
index 81aafa0..1f01a44 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -174,6 +174,11 @@ const char *path(const char *pathname);
#define qemu_isascii(c)isascii((unsigned char)(c))
#define qemu_toascii(c)toascii((unsigned char)(c))
+#ifdef _WIN32
+/* ffs() in oslib-win32.c for WIN32, strings.h for the rest of the world */
+int ffs(int i);
+#endif
+
void *qemu_malloc(size_t size);
void *qemu_realloc(void *ptr, size_t size);
void *qemu_mallocz(size_t size);
diff --git a/qemu-img.c b/qemu-img.c
index 578b8eb..5b2bed3 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -24,6 +24,7 @@
#include "qemu-common.h"
#include "qemu-option.h"
#include "osdep.h"
+#include "sysemu.h"
#include "block_int.h"
#include
diff --git a/qemu-os-posix.h b/qemu-os-posix.h
index ed5c058..353f878 100644
--- a/qemu-os-posix.h
+++ b/qemu-os-posix.h
@@ -36,4 +36,7 @@ void os_setup_signal_handling(void);
void os_daemonize(void);
void os_setup_post(void);
+typedef struct timeval qemu_timeval;
+#define qemu_gettimeofday(tp) gettimeofday(tp, NULL)
+
#endif
diff --git a/qemu-os-win32.h b/qemu-os-win32.h
index c63778d..1a07e5e 100644
--- a/qemu-os-win32.h
+++ b/qemu-os-win32.h
@@ -52,4 +52,12 @@ static inline void os_set_proc_name(const char *dummy) {}
# define EPROTONOSUPPORT EINVAL
#endif
+int setenv(const char *name, const char *value, int overwrite);
+
+typedef struct {
+long tv_sec;
+long tv_usec;
+} qemu_timeval;
+int qemu_gettimeofday(qemu_timeval *tp);
+
#endif
diff --git a/qemu-to
[Qemu-devel] [PATCH 7/9] Separate qemu_pidfile() into OS specific versions
From: Jes Sorensen
Signed-off-by: Jes Sorensen
---
os-posix.c | 21 +
os-win32.c | 24
osdep.c| 38 --
3 files changed, 45 insertions(+), 38 deletions(-)
diff --git a/os-posix.c b/os-posix.c
index 612b641..38c29d1 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -361,3 +361,24 @@ int qemu_eventfd(int fds[2])
return qemu_pipe(fds);
}
+
+int qemu_create_pidfile(const char *filename)
+{
+char buffer[128];
+int len;
+int fd;
+
+fd = qemu_open(filename, O_RDWR | O_CREAT, 0600);
+if (fd == -1) {
+return -1;
+}
+if (lockf(fd, F_TLOCK, 0) == -1) {
+return -1;
+}
+len = snprintf(buffer, sizeof(buffer), "%ld\n", (long)getpid());
+if (write(fd, buffer, len) != len) {
+return -1;
+}
+
+return 0;
+}
diff --git a/os-win32.c b/os-win32.c
index 3c6f50f..566d5e9 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -240,3 +240,27 @@ void os_pidfile_error(void)
{
fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno));
}
+
+int qemu_create_pidfile(const char *filename)
+{
+char buffer[128];
+int len;
+HANDLE file;
+OVERLAPPED overlap;
+BOOL ret;
+memset(&overlap, 0, sizeof(overlap));
+
+file = CreateFile(filename, GENERIC_WRITE, FILE_SHARE_READ, NULL,
+ OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
+
+if (file == INVALID_HANDLE_VALUE) {
+return -1;
+}
+len = snprintf(buffer, sizeof(buffer), "%ld\n", (long)getpid());
+ret = WriteFileEx(file, (LPCVOID)buffer, (DWORD)len,
+ &overlap, NULL);
+if (ret == 0) {
+return -1;
+}
+return 0;
+}
diff --git a/osdep.c b/osdep.c
index b1664ac..0d48561 100644
--- a/osdep.c
+++ b/osdep.c
@@ -73,44 +73,6 @@ int qemu_madvise(void *addr, size_t len, int advice)
#endif
}
-int qemu_create_pidfile(const char *filename)
-{
-char buffer[128];
-int len;
-#ifndef _WIN32
-int fd;
-
-fd = qemu_open(filename, O_RDWR | O_CREAT, 0600);
-if (fd == -1)
-return -1;
-
-if (lockf(fd, F_TLOCK, 0) == -1)
-return -1;
-
-len = snprintf(buffer, sizeof(buffer), "%ld\n", (long)getpid());
-if (write(fd, buffer, len) != len)
-return -1;
-#else
-HANDLE file;
-OVERLAPPED overlap;
-BOOL ret;
-memset(&overlap, 0, sizeof(overlap));
-
-file = CreateFile(filename, GENERIC_WRITE, FILE_SHARE_READ, NULL,
- OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
-
-if (file == INVALID_HANDLE_VALUE)
- return -1;
-
-len = snprintf(buffer, sizeof(buffer), "%ld\n", (long)getpid());
-ret = WriteFileEx(file, (LPCVOID)buffer, (DWORD)len,
- &overlap, NULL);
-if (ret == 0)
- return -1;
-#endif
-return 0;
-}
-
/*
* Opens a file with FD_CLOEXEC set
--
1.7.2.3
[Qemu-devel] [PATCH 4/9] We only support eventfd under POSIX, move qemu_eventfd() to os-posix.c
From: Jes Sorensen
Signed-off-by: Jes Sorensen
---
os-posix.c | 32
osdep.c| 34 --
2 files changed, 32 insertions(+), 34 deletions(-)
diff --git a/os-posix.c b/os-posix.c
index 6321e99..612b641 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -43,6 +43,10 @@
#include
#endif
+#ifdef CONFIG_EVENTFD
+#include
+#endif
+
static struct passwd *user_pwd;
static const char *chroot_dir;
static int daemonize;
@@ -329,3 +333,31 @@ void os_set_line_buffering(void)
{
setvbuf(stdout, NULL, _IOLBF, 0);
}
+
+/*
+ * Creates an eventfd that looks like a pipe and has EFD_CLOEXEC set.
+ */
+int qemu_eventfd(int fds[2])
+{
+#ifdef CONFIG_EVENTFD
+int ret;
+
+ret = eventfd(0, 0);
+if (ret >= 0) {
+fds[0] = ret;
+qemu_set_cloexec(ret);
+if ((fds[1] = dup(ret)) == -1) {
+close(ret);
+return -1;
+}
+qemu_set_cloexec(fds[1]);
+return 0;
+}
+
+if (errno != ENOSYS) {
+return -1;
+}
+#endif
+
+return qemu_pipe(fds);
+}
diff --git a/osdep.c b/osdep.c
index 926c8ad..cb12e5f 100644
--- a/osdep.c
+++ b/osdep.c
@@ -44,10 +44,6 @@
extern int madvise(caddr_t, size_t, int);
#endif
-#ifdef CONFIG_EVENTFD
-#include
-#endif
-
#ifdef _WIN32
#include
#elif defined(CONFIG_BSD)
@@ -207,36 +203,6 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t
count)
return total;
}
-#ifndef _WIN32
-/*
- * Creates an eventfd that looks like a pipe and has EFD_CLOEXEC set.
- */
-int qemu_eventfd(int fds[2])
-{
-#ifdef CONFIG_EVENTFD
-int ret;
-
-ret = eventfd(0, 0);
-if (ret >= 0) {
-fds[0] = ret;
-qemu_set_cloexec(ret);
-if ((fds[1] = dup(ret)) == -1) {
-close(ret);
-return -1;
-}
-qemu_set_cloexec(fds[1]);
-return 0;
-}
-
-if (errno != ENOSYS) {
-return -1;
-}
-#endif
-
-return qemu_pipe(fds);
-}
-#endif
-
/*
* Opens a socket with FD_CLOEXEC set
*/
--
1.7.2.3
[Qemu-devel] [PATCH 6/9] Do not redefine reserved key-words TRUE/FALSE
From: Jes Sorensen
TRUE/FALSE are generally reserved keywords and shouldn't be defined in
a driver like this. Rename the macros to SDP_TRUE and SDP_FALSE
respectively.
Signed-off-by: Jes Sorensen
---
hw/bt-sdp.c | 20 ++--
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/hw/bt-sdp.c b/hw/bt-sdp.c
index cc0bf2f..cdf2d95 100644
--- a/hw/bt-sdp.c
+++ b/hw/bt-sdp.c
@@ -786,11 +786,11 @@ static void sdp_service_db_build(struct
bt_l2cap_sdp_state_s *sdp,
.type = SDP_DTYPE_UUID | SDP_DSIZE_16, \
.value.uint = val, \
},
-#define TRUE { \
+#define SDP_TRUE { \
.type = SDP_DTYPE_BOOL | SDP_DSIZE_1,\
.value.uint = 1, \
},
-#define FALSE { \
+#define SDP_FALSE { \
.type = SDP_DTYPE_BOOL | SDP_DSIZE_1,\
.value.uint = 0, \
},
@@ -842,8 +842,8 @@ SERVICE(hid,
/* TODO: extract from l2cap_device->device.class[0] */
ATTRIBUTE(DEVICE_SUBCLASS, UINT8(0x40))
ATTRIBUTE(COUNTRY_CODE,UINT8(0x15))
-ATTRIBUTE(VIRTUAL_CABLE, TRUE)
-ATTRIBUTE(RECONNECT_INITIATE, FALSE)
+ATTRIBUTE(VIRTUAL_CABLE, SDP_TRUE)
+ATTRIBUTE(RECONNECT_INITIATE, SDP_FALSE)
/* TODO: extract from hid->usbdev->report_desc */
ATTRIBUTE(DESCRIPTOR_LIST, LIST(
LIST(UINT8(0x22) ARRAY(
@@ -883,12 +883,12 @@ SERVICE(hid,
ATTRIBUTE(LANG_ID_BASE_LIST, LIST(
LIST(UINT16(0x0409) UINT16(0x0100))
))
-ATTRIBUTE(SDP_DISABLE, FALSE)
-ATTRIBUTE(BATTERY_POWER, TRUE)
-ATTRIBUTE(REMOTE_WAKEUP, TRUE)
-ATTRIBUTE(BOOT_DEVICE, TRUE) /* XXX: untested */
+ATTRIBUTE(SDP_DISABLE, SDP_FALSE)
+ATTRIBUTE(BATTERY_POWER, SDP_TRUE)
+ATTRIBUTE(REMOTE_WAKEUP, SDP_TRUE)
+ATTRIBUTE(BOOT_DEVICE, SDP_TRUE) /* XXX: untested */
ATTRIBUTE(SUPERVISION_TIMEOUT, UINT16(0x0c80))
-ATTRIBUTE(NORMALLY_CONNECTABLE,TRUE)
+ATTRIBUTE(NORMALLY_CONNECTABLE,SDP_TRUE)
ATTRIBUTE(PROFILE_VERSION, UINT16(0x0100))
)
@@ -936,7 +936,7 @@ SERVICE(pnp,
/* Profile specific */
ATTRIBUTE(SPECIFICATION_ID, UINT16(0x0100))
ATTRIBUTE(VERSION, UINT16(0x0100))
-ATTRIBUTE(PRIMARY_RECORD, TRUE)
+ATTRIBUTE(PRIMARY_RECORD, SDP_TRUE)
)
static int bt_l2cap_sdp_new_ch(struct bt_l2cap_device_s *dev,
--
1.7.2.3
[Qemu-devel] [PATCH 9/9] Remove unncessary includes
From: Jes Sorensen No need to include stdlib.h for BSD as it is included by qemu-common.h, windows.h is handled by sysemu.h and osdep.c no longer needs malloc.h Signed-off-by: Jes Sorensen --- osdep.c |8 1 files changed, 0 insertions(+), 8 deletions(-) diff --git a/osdep.c b/osdep.c index 0d48561..327583b 100644 --- a/osdep.c +++ b/osdep.c @@ -44,14 +44,6 @@ extern int madvise(caddr_t, size_t, int); #endif -#ifdef _WIN32 -#include -#elif defined(CONFIG_BSD) -#include -#else -#include -#endif - #include "qemu-common.h" #include "trace.h" #include "sysemu.h" -- 1.7.2.3
[Qemu-devel] Re: [PATCH 5/9] Move qemu_gettimeofday() to OS specific files
On 10/16/10 21:32, Blue Swirl wrote: > On Sat, Oct 16, 2010 at 4:04 PM, wrote: >> From: Jes Sorensen >> >> In addition add sysemu.h includes to file requiring a prototype for >> ffs() > > There are probably a lot more files which would need that: > /src/qemu/hw/sd.c: In function 'sd_normal_command': > /src/qemu/hw/sd.c:738:13: error: implicit declaration of function > 'ffs' [-Werror=implicit-function-declaration] > /src/qemu/hw/max7310.c: In function 'max7310_tx': > /src/qemu/hw/max7310.c:94:13: error: implicit declaration of function > 'ffs' [-Werror=implicit-function-declaration] > /src/qemu/hw/unin_pci.c: In function 'unin_get_config_reg': > /src/qemu/hw/unin_pci.c:101:9: error: implicit declaration of function > 'ffs' [-Werror=implicit-function-declaration] > > Perhaps the prototype should be added someplace else. I guess we'll have to bite the bullet. I don't really like it, but I moved it to qemu-common.h to be consistent with the POSIX code. POSIX relies on ffs() to be provided by strings.h which we include in qemu-common.h Should build (I hope) in the next patch. I tried building arm-softmmu here but it wouldn't build for me at all due to other things so I couldn't test it. Cheers, Jes
[Qemu-devel] [PATCH 1/9] Move QEMU OS dependant library functions to OS specific files
From: Jes Sorensen
This moves library functions used by both QEMU and the QEMU tools,
such as qemu-img, qemu-nbd etc. from osdep.c to oslib-{posix,win32}.c
In addition it introduces oslib-obj.y to the Makefile set to be
included by the various targets, instead of relying on these library
functions magically getting included via block-obj-y.
Signed-off-by: Jes Sorensen
---
Makefile |6 ++--
Makefile.objs |9 +-
osdep.c | 85 -
oslib-posix.c | 74 +
oslib-win32.c | 73 +
5 files changed, 158 insertions(+), 89 deletions(-)
create mode 100644 oslib-posix.c
create mode 100644 oslib-win32.c
diff --git a/Makefile b/Makefile
index 252c817..0b3751d 100644
--- a/Makefile
+++ b/Makefile
@@ -129,11 +129,11 @@ version-obj-$(CONFIG_WIN32) += version.o
qemu-img.o: qemu-img-cmds.h
qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o: $(GENERATED_HEADERS)
-qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(trace-obj-y)
$(block-obj-y) $(qobject-obj-y) $(version-obj-y)
+qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(oslib-obj-y)
$(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y)
-qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(trace-obj-y)
$(block-obj-y) $(qobject-obj-y) $(version-obj-y)
+qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(oslib-obj-y)
$(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y)
-qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(trace-obj-y)
$(block-obj-y) $(qobject-obj-y) $(version-obj-y)
+qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(oslib-obj-y)
$(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y)
qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@," GEN $@")
diff --git a/Makefile.objs b/Makefile.objs
index 816194a..ec1a09a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -5,10 +5,16 @@ qobject-obj-y += qjson.o json-lexer.o json-streamer.o
json-parser.o
qobject-obj-y += qerror.o
###
+# oslib-obj-y is code depending on the OS (win32 vs posix)
+oslib-obj-y = osdep.o
+oslib-obj-$(CONFIG_WIN32) += oslib-win32.o
+oslib-obj-$(CONFIG_POSIX) += oslib-posix.o
+
+###
# block-obj-y is code used by both qemu system emulation and qemu-img
block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
-block-obj-y += nbd.o block.o aio.o aes.o osdep.o qemu-config.o
+block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o
block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
@@ -50,6 +56,7 @@ common-obj-y += $(net-obj-y)
common-obj-y += $(qobject-obj-y)
common-obj-$(CONFIG_LINUX) += $(fsdev-obj-$(CONFIG_LINUX))
common-obj-y += readline.o console.o cursor.o async.o qemu-error.o
+common-obj-y += $(oslib-obj-y)
common-obj-$(CONFIG_WIN32) += os-win32.o
common-obj-$(CONFIG_POSIX) += os-posix.o
diff --git a/osdep.c b/osdep.c
index 2e05b21..581768a 100644
--- a/osdep.c
+++ b/osdep.c
@@ -61,91 +61,6 @@ extern int madvise(caddr_t, size_t, int);
#include "sysemu.h"
#include "qemu_socket.h"
-#if !defined(_POSIX_C_SOURCE) || defined(_WIN32) || defined(__sun__)
-static void *oom_check(void *ptr)
-{
-if (ptr == NULL) {
-#if defined(_WIN32)
-fprintf(stderr, "Failed to allocate memory: %lu\n", GetLastError());
-#else
-fprintf(stderr, "Failed to allocate memory: %s\n", strerror(errno));
-#endif
-abort();
-}
-return ptr;
-}
-#endif
-
-#if defined(_WIN32)
-void *qemu_memalign(size_t alignment, size_t size)
-{
-void *ptr;
-
-if (!size) {
-abort();
-}
-ptr = oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
-trace_qemu_memalign(alignment, size, ptr);
-return ptr;
-}
-
-void *qemu_vmalloc(size_t size)
-{
-void *ptr;
-
-/* FIXME: this is not exactly optimal solution since VirtualAlloc
- has 64Kb granularity, but at least it guarantees us that the
- memory is page aligned. */
-if (!size) {
-abort();
-}
-ptr = oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
-trace_qemu_vmalloc(size, ptr);
-return ptr;
-}
-
-void qemu_vfree(void *ptr)
-{
-trace_qemu_vfree(ptr);
-VirtualFree(ptr, 0, MEM_RELEASE);
-}
-
-#else
-
-void *qemu_memalign(size_t alignment, size_t size)
-{
-void *ptr;
-#if defined(_POSIX_C_SOURCE) && !defined(__sun__)
-int ret;
-ret = posix_memalign(&ptr, alignment, size);
-if (ret != 0) {
-fprintf(stderr, "Failed to allocate %zu B: %s\n",
-size, strerror(ret));
-abort();
-}
-#elif defined(CON
