* [Qemu-devel] [PULL 1/9] Remove the deprecated -balloon option
2018-08-31 8:38 [Qemu-devel] [PULL 0/9] Removal of deprecated options and improvements for qtests Thomas Huth
@ 2018-08-31 8:38 ` Thomas Huth
2018-08-31 8:38 ` [Qemu-devel] [PULL 2/9] Remove the deprecated -nodefconfig option Thomas Huth
` (7 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2018-08-31 8:38 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Paolo Bonzini, Laurent Vivier, Eric Blake, Marc-André Lureau
The "-balloon" option has been replaced by "-device virtio-balloon".
It's been marked as deprecated since two releases, and nobody
complained, so let's remove it now.
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>
Acked-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
docs/virtio-balloon-stats.txt | 6 +++---
qemu-deprecated.texi | 5 -----
qemu-options.hx | 10 ----------
vl.c | 36 ------------------------------------
4 files changed, 3 insertions(+), 54 deletions(-)
diff --git a/docs/virtio-balloon-stats.txt b/docs/virtio-balloon-stats.txt
index 9985e1d..1732cc8 100644
--- a/docs/virtio-balloon-stats.txt
+++ b/docs/virtio-balloon-stats.txt
@@ -61,9 +61,9 @@ It's also important to note the following:
respond to the request the timer will never be re-armed, which has
the same effect as disabling polling
-Here are a few examples. QEMU is started with '-balloon virtio', which
-generates '/machine/peripheral-anon/device[1]' as the QOM path for the
-balloon device.
+Here are a few examples. QEMU is started with '-device virtio-balloon',
+which generates '/machine/peripheral-anon/device[1]' as the QOM path for
+the balloon device.
Enable polling with 2 seconds interval:
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 1b9c007..98f5062 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -86,11 +86,6 @@ enabled via the ``-machine usb=on'' argument.
The ``-nodefconfig`` argument is a synonym for ``-no-user-config``.
-@subsection -balloon (since 2.12.0)
-
-The @option{--balloon virtio} argument has been superseded by
-@option{--device virtio-balloon}.
-
@subsection -fsdev handle (since 2.12.0)
The ``handle'' fsdev backend does not support symlinks and causes the 9p
diff --git a/qemu-options.hx b/qemu-options.hx
index 654ef48..ccf7155 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -454,16 +454,6 @@ modprobe i810_audio clocking=48000
@end example
ETEXI
-DEF("balloon", HAS_ARG, QEMU_OPTION_balloon,
- "-balloon virtio[,addr=str]\n"
- " enable virtio balloon device (deprecated)\n", QEMU_ARCH_ALL)
-STEXI
-@item -balloon virtio[,addr=@var{addr}]
-@findex -balloon
-Enable virtio balloon device, optionally with PCI address @var{addr}. This
-option is deprecated, use @option{-device virtio-balloon} instead.
-ETEXI
-
DEF("device", HAS_ARG, QEMU_OPTION_device,
"-device driver[,prop[=value][,...]]\n"
" add device (based on driver)\n"
diff --git a/vl.c b/vl.c
index 5ba06ad..674c42f 100644
--- a/vl.c
+++ b/vl.c
@@ -2127,36 +2127,6 @@ static void parse_display(const char *p)
}
}
-static int balloon_parse(const char *arg)
-{
- QemuOpts *opts;
-
- warn_report("This option is deprecated. "
- "Use '--device virtio-balloon' to enable the balloon device.");
-
- if (strcmp(arg, "none") == 0) {
- return 0;
- }
-
- if (!strncmp(arg, "virtio", 6)) {
- if (arg[6] == ',') {
- /* have params -> parse them */
- opts = qemu_opts_parse_noisily(qemu_find_opts("device"), arg + 7,
- false);
- if (!opts)
- return -1;
- } else {
- /* create empty opts */
- opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
- &error_abort);
- }
- qemu_opt_set(opts, "driver", "virtio-balloon", &error_abort);
- return 0;
- }
-
- return -1;
-}
-
char *qemu_find_file(int type, const char *name)
{
int i;
@@ -3660,12 +3630,6 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_no_hpet:
no_hpet = 1;
break;
- case QEMU_OPTION_balloon:
- if (balloon_parse(optarg) < 0) {
- error_report("unknown -balloon argument %s", optarg);
- exit(1);
- }
- break;
case QEMU_OPTION_no_reboot:
no_reboot = 1;
break;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL 2/9] Remove the deprecated -nodefconfig option
2018-08-31 8:38 [Qemu-devel] [PULL 0/9] Removal of deprecated options and improvements for qtests Thomas Huth
2018-08-31 8:38 ` [Qemu-devel] [PULL 1/9] Remove the deprecated -balloon option Thomas Huth
@ 2018-08-31 8:38 ` Thomas Huth
2018-08-31 8:38 ` [Qemu-devel] [PULL 3/9] Remove the deprecated options -startdate, -localtime and -rtc-td-hack Thomas Huth
` (6 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2018-08-31 8:38 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Paolo Bonzini, Laurent Vivier, Eric Blake, Marc-André Lureau
It's the same as -no-user-config and marked as deprecated since three
releases already. Time to remove it now.
Acked-by: Peter Krempa <pkrempa@redhat.com>
Acked-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
docs/interop/live-block-operations.rst | 4 ++--
qemu-deprecated.texi | 4 ----
qemu-options.hx | 4 ++--
vl.c | 2 --
4 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/docs/interop/live-block-operations.rst b/docs/interop/live-block-operations.rst
index 734252b..48afdc7 100644
--- a/docs/interop/live-block-operations.rst
+++ b/docs/interop/live-block-operations.rst
@@ -129,7 +129,7 @@ To show some example invocations of command-line, we will use the
following invocation of QEMU, with a QMP server running over UNIX
socket::
- $ ./x86_64-softmmu/qemu-system-x86_64 -display none -nodefconfig \
+ $ ./x86_64-softmmu/qemu-system-x86_64 -display none -no-user-config \
-M q35 -nodefaults -m 512 \
-blockdev node-name=node-A,driver=qcow2,file.driver=file,file.node-name=file,file.filename=./a.qcow2 \
-device virtio-blk,drive=node-A,id=virtio0 \
@@ -694,7 +694,7 @@ instance, with the following invocation. (As noted earlier, for
simplicity's sake, the destination QEMU is started on the same host, but
it could be located elsewhere)::
- $ ./x86_64-softmmu/qemu-system-x86_64 -display none -nodefconfig \
+ $ ./x86_64-softmmu/qemu-system-x86_64 -display none -no-user-config \
-M q35 -nodefaults -m 512 \
-blockdev node-name=node-TargetDisk,driver=qcow2,file.driver=file,file.node-name=file,file.filename=./target-disk.qcow2 \
-device virtio-blk,drive=node-TargetDisk,id=virtio0 \
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 98f5062..19c8ae2 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -82,10 +82,6 @@ would automatically enable USB support on the machine type.
If using the new syntax, USB support must be explicitly
enabled via the ``-machine usb=on'' argument.
-@subsection -nodefconfig (since 2.11.0)
-
-The ``-nodefconfig`` argument is a synonym for ``-no-user-config``.
-
@subsection -fsdev handle (since 2.12.0)
The ``handle'' fsdev backend does not support symlinks and causes the 9p
diff --git a/qemu-options.hx b/qemu-options.hx
index ccf7155..d9be20b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3833,8 +3833,7 @@ Write device configuration to @var{file}. The @var{file} can be either filename
command line and device configuration into file or dash @code{-}) character to print the
output to stdout. This can be later used as input file for @code{-readconfig} option.
ETEXI
-HXCOMM Deprecated, same as -no-user-config
-DEF("nodefconfig", 0, QEMU_OPTION_nodefconfig, "", QEMU_ARCH_ALL)
+
DEF("no-user-config", 0, QEMU_OPTION_nouserconfig,
"-no-user-config\n"
" do not load default user-provided config files at startup\n",
@@ -3845,6 +3844,7 @@ STEXI
The @code{-no-user-config} option makes QEMU not load any of the user-provided
config files on @var{sysconfdir}.
ETEXI
+
DEF("trace", HAS_ARG, QEMU_OPTION_trace,
"-trace [[enable=]<pattern>][,events=<file>][,file=<file>]\n"
" specify tracing options\n",
diff --git a/vl.c b/vl.c
index 674c42f..386c71d 100644
--- a/vl.c
+++ b/vl.c
@@ -2999,7 +2999,6 @@ int main(int argc, char **argv, char **envp)
popt = lookup_opt(argc, argv, &optarg, &optind);
switch (popt->index) {
- case QEMU_OPTION_nodefconfig:
case QEMU_OPTION_nouserconfig:
userconfig = false;
break;
@@ -3927,7 +3926,6 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_enable_sync_profile:
qsp_enable();
break;
- case QEMU_OPTION_nodefconfig:
case QEMU_OPTION_nouserconfig:
/* Nothing to be parsed here. Especially, do not error out below. */
break;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL 3/9] Remove the deprecated options -startdate, -localtime and -rtc-td-hack
2018-08-31 8:38 [Qemu-devel] [PULL 0/9] Removal of deprecated options and improvements for qtests Thomas Huth
2018-08-31 8:38 ` [Qemu-devel] [PULL 1/9] Remove the deprecated -balloon option Thomas Huth
2018-08-31 8:38 ` [Qemu-devel] [PULL 2/9] Remove the deprecated -nodefconfig option Thomas Huth
@ 2018-08-31 8:38 ` Thomas Huth
2018-08-31 8:38 ` [Qemu-devel] [PULL 4/9] net: Remove the deprecated -tftp, -bootp, -redir and -smb options Thomas Huth
` (5 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2018-08-31 8:38 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Paolo Bonzini, Laurent Vivier, Eric Blake, Marc-André Lureau
Deprecated since two releases, nobody complained, thus it's time to
remove them now.
Acked-by: Peter Krempa <pkrempa@redhat.com>
Acked-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
qemu-deprecated.texi | 13 ---------
qemu-options.hx | 7 -----
vl.c | 76 +++++++++++++++-------------------------------------
3 files changed, 22 insertions(+), 74 deletions(-)
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 19c8ae2..ca52e83 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -96,19 +96,6 @@ The @code{--no-frame} argument works with SDL 1.2 only. The other user
interfaces never implemented this in the first place. So this will be
removed together with SDL 1.2 support.
-@subsection -rtc-td-hack (since 2.12.0)
-
-The @code{-rtc-td-hack} option has been replaced by
-@code{-rtc driftfix=slew}.
-
-@subsection -localtime (since 2.12.0)
-
-The @code{-localtime} option has been replaced by @code{-rtc base=localtime}.
-
-@subsection -startdate (since 2.12.0)
-
-The @code{-startdate} option has been replaced by @code{-rtc base=@var{date}}.
-
@subsection -virtioconsole (since 3.0.0)
Option @option{-virtioconsole} has been replaced by
diff --git a/qemu-options.hx b/qemu-options.hx
index d9be20b..7ca539a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1711,9 +1711,6 @@ Windows 2000 is installed, you no longer need this option (this option
slows down the IDE transfers).
ETEXI
-HXCOMM Deprecated by -rtc
-DEF("rtc-td-hack", 0, QEMU_OPTION_rtc_td_hack, "", QEMU_ARCH_I386)
-
DEF("no-fd-bootchk", 0, QEMU_OPTION_no_fd_bootchk,
"-no-fd-bootchk disable boot signature checking for floppy disks\n",
QEMU_ARCH_I386)
@@ -3471,10 +3468,6 @@ ETEXI
HXCOMM Silently ignored for compatibility
DEF("clock", HAS_ARG, QEMU_OPTION_clock, "", QEMU_ARCH_ALL)
-HXCOMM Options deprecated by -rtc
-DEF("localtime", 0, QEMU_OPTION_localtime, "", QEMU_ARCH_ALL)
-DEF("startdate", HAS_ARG, QEMU_OPTION_startdate, "", QEMU_ARCH_ALL)
-
DEF("rtc", HAS_ARG, QEMU_OPTION_rtc, \
"-rtc [base=utc|localtime|date][,clock=host|rt|vm][,driftfix=none|slew]\n" \
" set the RTC base and clock, enable drift fix for clock ticks (x86 only)\n",
diff --git a/vl.c b/vl.c
index 386c71d..cfeee0d 100644
--- a/vl.c
+++ b/vl.c
@@ -823,44 +823,33 @@ int qemu_timedate_diff(struct tm *tm)
return seconds - qemu_time();
}
-static void configure_rtc_date_offset(const char *startdate, int legacy)
+static void configure_rtc_date_offset(const char *startdate)
{
time_t rtc_start_date;
struct tm tm;
- if (!strcmp(startdate, "now") && legacy) {
- rtc_date_offset = -1;
+ if (sscanf(startdate, "%d-%d-%dT%d:%d:%d", &tm.tm_year, &tm.tm_mon,
+ &tm.tm_mday, &tm.tm_hour, &tm.tm_min, &tm.tm_sec) == 6) {
+ /* OK */
+ } else if (sscanf(startdate, "%d-%d-%d",
+ &tm.tm_year, &tm.tm_mon, &tm.tm_mday) == 3) {
+ tm.tm_hour = 0;
+ tm.tm_min = 0;
+ tm.tm_sec = 0;
} else {
- if (sscanf(startdate, "%d-%d-%dT%d:%d:%d",
- &tm.tm_year,
- &tm.tm_mon,
- &tm.tm_mday,
- &tm.tm_hour,
- &tm.tm_min,
- &tm.tm_sec) == 6) {
- /* OK */
- } else if (sscanf(startdate, "%d-%d-%d",
- &tm.tm_year,
- &tm.tm_mon,
- &tm.tm_mday) == 3) {
- tm.tm_hour = 0;
- tm.tm_min = 0;
- tm.tm_sec = 0;
- } else {
- goto date_fail;
- }
- tm.tm_year -= 1900;
- tm.tm_mon--;
- rtc_start_date = mktimegm(&tm);
- if (rtc_start_date == -1) {
- date_fail:
- error_report("invalid date format");
- error_printf("valid formats: "
- "'2006-06-17T16:01:21' or '2006-06-17'\n");
- exit(1);
- }
- rtc_date_offset = qemu_time() - rtc_start_date;
+ goto date_fail;
+ }
+ tm.tm_year -= 1900;
+ tm.tm_mon--;
+ rtc_start_date = mktimegm(&tm);
+ if (rtc_start_date == -1) {
+ date_fail:
+ error_report("invalid date format");
+ error_printf("valid formats: "
+ "'2006-06-17T16:01:21' or '2006-06-17'\n");
+ exit(1);
}
+ rtc_date_offset = qemu_time() - rtc_start_date;
}
static void configure_rtc(QemuOpts *opts)
@@ -878,7 +867,7 @@ static void configure_rtc(QemuOpts *opts)
"-rtc base=localtime");
replay_add_blocker(blocker);
} else {
- configure_rtc_date_offset(value, 0);
+ configure_rtc_date_offset(value);
}
}
value = qemu_opt_get(opts, "clock");
@@ -3269,11 +3258,6 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_k:
keyboard_layout = optarg;
break;
- case QEMU_OPTION_localtime:
- rtc_utc = 0;
- warn_report("This option is deprecated, "
- "use '-rtc base=localtime' instead.");
- break;
case QEMU_OPTION_vga:
vga_model = optarg;
default_vga = 0;
@@ -3526,18 +3510,6 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_win2k_hack:
win2k_install_hack = 1;
break;
- case QEMU_OPTION_rtc_td_hack: {
- static GlobalProperty slew_lost_ticks = {
- .driver = "mc146818rtc",
- .property = "lost_tick_policy",
- .value = "slew",
- };
-
- qdev_prop_register_global(&slew_lost_ticks);
- warn_report("This option is deprecated, "
- "use '-rtc driftfix=slew' instead.");
- break;
- }
case QEMU_OPTION_acpitable:
opts = qemu_opts_parse_noisily(qemu_find_opts("acpi"),
optarg, true);
@@ -3723,10 +3695,6 @@ int main(int argc, char **argv, char **envp)
*/
warn_report("This option is ignored and will be removed soon");
break;
- case QEMU_OPTION_startdate:
- warn_report("This option is deprecated, use '-rtc base=' instead.");
- configure_rtc_date_offset(optarg, 1);
- break;
case QEMU_OPTION_rtc:
opts = qemu_opts_parse_noisily(qemu_find_opts("rtc"), optarg,
false);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL 4/9] net: Remove the deprecated -tftp, -bootp, -redir and -smb options
2018-08-31 8:38 [Qemu-devel] [PULL 0/9] Removal of deprecated options and improvements for qtests Thomas Huth
` (2 preceding siblings ...)
2018-08-31 8:38 ` [Qemu-devel] [PULL 3/9] Remove the deprecated options -startdate, -localtime and -rtc-td-hack Thomas Huth
@ 2018-08-31 8:38 ` Thomas Huth
2018-08-31 8:38 ` [Qemu-devel] [PULL 5/9] tests/libqos: Utilize newer glib spawn check Thomas Huth
` (4 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2018-08-31 8:38 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Paolo Bonzini, Laurent Vivier, Eric Blake, Marc-André Lureau
These options likely do not work as expected as soon as the user
tries to use more than one network interface at once. The parameters
have been marked as deprecated since QEMU v2.6, so users had plenty
of time to move their scripts to the new syntax. Time to remove the
old parameters now.
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Acked-by: Peter Krempa <pkrempa@redhat.com>
Acked-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
include/net/net.h | 3 --
include/net/slirp.h | 4 --
net/slirp.c | 132 +++++++++++----------------------------------------
os-posix.c | 8 ----
qemu-deprecated.texi | 34 -------------
qemu-options.hx | 15 ------
vl.c | 18 -------
7 files changed, 29 insertions(+), 185 deletions(-)
diff --git a/include/net/net.h b/include/net/net.h
index 1425960..7936d53 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -201,9 +201,6 @@ extern NICInfo nd_table[MAX_NICS];
extern const char *host_net_devices[];
/* from net.c */
-extern const char *legacy_tftp_prefix;
-extern const char *legacy_bootp_filename;
-
int net_client_parse(QemuOptsList *opts_list, const char *str);
int net_init_clients(Error **errp);
void net_check_clients(void);
diff --git a/include/net/slirp.h b/include/net/slirp.h
index 4d63d74..bad3e1e 100644
--- a/include/net/slirp.h
+++ b/include/net/slirp.h
@@ -30,10 +30,6 @@
void hmp_hostfwd_add(Monitor *mon, const QDict *qdict);
void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict);
-int net_slirp_redir(const char *redir_str);
-
-int net_slirp_smb(const char *exported_dir);
-
void hmp_info_usernet(Monitor *mon, const QDict *qdict);
#endif
diff --git a/net/slirp.c b/net/slirp.c
index 1e14318..c18060f 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -67,13 +67,11 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
/* slirp network adapter */
#define SLIRP_CFG_HOSTFWD 1
-#define SLIRP_CFG_LEGACY 2
struct slirp_config_str {
struct slirp_config_str *next;
int flags;
char str[1024];
- int legacy_format;
};
typedef struct SlirpState {
@@ -87,19 +85,13 @@ typedef struct SlirpState {
} SlirpState;
static struct slirp_config_str *slirp_configs;
-const char *legacy_tftp_prefix;
-const char *legacy_bootp_filename;
static QTAILQ_HEAD(slirp_stacks, SlirpState) slirp_stacks =
QTAILQ_HEAD_INITIALIZER(slirp_stacks);
-static int slirp_hostfwd(SlirpState *s, const char *redir_str,
- int legacy_format, Error **errp);
-static int slirp_guestfwd(SlirpState *s, const char *config_str,
- int legacy_format, Error **errp);
+static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp);
+static int slirp_guestfwd(SlirpState *s, const char *config_str, Error **errp);
#ifndef _WIN32
-static const char *legacy_smb_export;
-
static int slirp_smb(SlirpState *s, const char *exported_dir,
struct in_addr vserver_addr, Error **errp);
static void slirp_smb_cleanup(SlirpState *s);
@@ -196,13 +188,6 @@ static int net_slirp_init(NetClientState *peer, const char *model,
return -1;
}
- if (!tftp_export) {
- tftp_export = legacy_tftp_prefix;
- }
- if (!bootfile) {
- bootfile = legacy_bootp_filename;
- }
-
if (vnetwork) {
if (get_str_sep(buf, sizeof(buf), &vnetwork, '/') < 0) {
if (!inet_aton(vnetwork, &net)) {
@@ -382,21 +367,16 @@ static int net_slirp_init(NetClientState *peer, const char *model,
for (config = slirp_configs; config; config = config->next) {
if (config->flags & SLIRP_CFG_HOSTFWD) {
- if (slirp_hostfwd(s, config->str,
- config->flags & SLIRP_CFG_LEGACY, errp) < 0) {
+ if (slirp_hostfwd(s, config->str, errp) < 0) {
goto error;
}
} else {
- if (slirp_guestfwd(s, config->str,
- config->flags & SLIRP_CFG_LEGACY, errp) < 0) {
+ if (slirp_guestfwd(s, config->str, errp) < 0) {
goto error;
}
}
}
#ifndef _WIN32
- if (!smb_export) {
- smb_export = legacy_smb_export;
- }
if (smb_export) {
if (slirp_smb(s, smb_export, smbsrv, errp) < 0) {
goto error;
@@ -506,8 +486,7 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
monitor_printf(mon, "invalid format\n");
}
-static int slirp_hostfwd(SlirpState *s, const char *redir_str,
- int legacy_format, Error **errp)
+static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp)
{
struct in_addr host_addr = { .s_addr = INADDR_ANY };
struct in_addr guest_addr = { .s_addr = 0 };
@@ -532,18 +511,16 @@ static int slirp_hostfwd(SlirpState *s, const char *redir_str,
goto fail_syntax;
}
- if (!legacy_format) {
- if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
- fail_reason = "Missing : separator";
- goto fail_syntax;
- }
- if (buf[0] != '\0' && !inet_aton(buf, &host_addr)) {
- fail_reason = "Bad host address";
- goto fail_syntax;
- }
+ if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
+ fail_reason = "Missing : separator";
+ goto fail_syntax;
+ }
+ if (buf[0] != '\0' && !inet_aton(buf, &host_addr)) {
+ fail_reason = "Bad host address";
+ goto fail_syntax;
}
- if (get_str_sep(buf, sizeof(buf), &p, legacy_format ? ':' : '-') < 0) {
+ if (get_str_sep(buf, sizeof(buf), &p, '-') < 0) {
fail_reason = "Bad host port separator";
goto fail_syntax;
}
@@ -602,35 +579,13 @@ void hmp_hostfwd_add(Monitor *mon, const QDict *qdict)
}
if (s) {
Error *err = NULL;
- if (slirp_hostfwd(s, redir_str, 0, &err) < 0) {
+ if (slirp_hostfwd(s, redir_str, &err) < 0) {
error_report_err(err);
}
}
}
-int net_slirp_redir(const char *redir_str)
-{
- struct slirp_config_str *config;
- Error *err = NULL;
- int res;
-
- if (QTAILQ_EMPTY(&slirp_stacks)) {
- config = g_malloc(sizeof(*config));
- pstrcpy(config->str, sizeof(config->str), redir_str);
- config->flags = SLIRP_CFG_HOSTFWD | SLIRP_CFG_LEGACY;
- config->next = slirp_configs;
- slirp_configs = config;
- return 0;
- }
-
- res = slirp_hostfwd(QTAILQ_FIRST(&slirp_stacks), redir_str, 1, &err);
- if (res < 0) {
- error_report_err(err);
- }
- return res;
-}
-
#ifndef _WIN32
/* automatic user mode samba server configuration */
@@ -746,28 +701,6 @@ static int slirp_smb(SlirpState* s, const char *exported_dir,
return 0;
}
-/* automatic user mode samba server configuration (legacy interface) */
-int net_slirp_smb(const char *exported_dir)
-{
- struct in_addr vserver_addr = { .s_addr = 0 };
-
- if (legacy_smb_export) {
- fprintf(stderr, "-smb given twice\n");
- return -1;
- }
- legacy_smb_export = exported_dir;
- if (!QTAILQ_EMPTY(&slirp_stacks)) {
- Error *err = NULL;
- int res = slirp_smb(QTAILQ_FIRST(&slirp_stacks), exported_dir,
- vserver_addr, &err);
- if (res < 0) {
- error_report_err(err);
- }
- return res;
- }
- return 0;
-}
-
#endif /* !defined(_WIN32) */
struct GuestFwd {
@@ -789,8 +722,7 @@ static void guestfwd_read(void *opaque, const uint8_t *buf, int size)
slirp_socket_recv(fwd->slirp, fwd->server, fwd->port, buf, size);
}
-static int slirp_guestfwd(SlirpState *s, const char *config_str,
- int legacy_format, Error **errp)
+static int slirp_guestfwd(SlirpState *s, const char *config_str, Error **errp)
{
struct in_addr server = { .s_addr = 0 };
struct GuestFwd *fwd;
@@ -800,26 +732,20 @@ static int slirp_guestfwd(SlirpState *s, const char *config_str,
int port;
p = config_str;
- if (legacy_format) {
- if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
- goto fail_syntax;
- }
- } else {
- if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
- goto fail_syntax;
- }
- if (strcmp(buf, "tcp") && buf[0] != '\0') {
- goto fail_syntax;
- }
- if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
- goto fail_syntax;
- }
- if (buf[0] != '\0' && !inet_aton(buf, &server)) {
- goto fail_syntax;
- }
- if (get_str_sep(buf, sizeof(buf), &p, '-') < 0) {
- goto fail_syntax;
- }
+ if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
+ goto fail_syntax;
+ }
+ if (strcmp(buf, "tcp") && buf[0] != '\0') {
+ goto fail_syntax;
+ }
+ if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
+ goto fail_syntax;
+ }
+ if (buf[0] != '\0' && !inet_aton(buf, &server)) {
+ goto fail_syntax;
+ }
+ if (get_str_sep(buf, sizeof(buf), &p, '-') < 0) {
+ goto fail_syntax;
}
port = strtol(buf, &end, 10);
if (*end != '\0' || port < 1 || port > 65535) {
diff --git a/os-posix.c b/os-posix.c
index 9ce6f74..8f39447 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -168,14 +168,6 @@ static bool os_parse_runas_uid_gid(const char *optarg)
int os_parse_cmd_args(int index, const char *optarg)
{
switch (index) {
-#ifdef CONFIG_SLIRP
- case QEMU_OPTION_smb:
- error_report("The -smb option is deprecated. "
- "Please use '-netdev user,smb=...' instead.");
- if (net_slirp_smb(optarg) < 0)
- exit(1);
- break;
-#endif
case QEMU_OPTION_runas:
user_pwd = getpwnam(optarg);
if (user_pwd) {
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index ca52e83..8a2e399 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -40,40 +40,6 @@ which is the default.
The ``-no-kvm'' argument is now a synonym for setting
``-machine accel=tcg''.
-@subsection -tftp (since 2.6.0)
-
-The ``-tftp /some/dir'' argument is replaced by either
-``-netdev user,id=x,tftp=/some/dir '' (for pluggable NICs, accompanied
-with ``-device ...,netdev=x''), or ``-nic user,tftp=/some/dir''
-(for embedded NICs). The new syntax allows different settings to be
-provided per NIC.
-
-@subsection -bootp (since 2.6.0)
-
-The ``-bootp /some/file'' argument is replaced by either
-``-netdev user,id=x,bootp=/some/file '' (for pluggable NICs, accompanied
-with ``-device ...,netdev=x''), or ``-nic user,bootp=/some/file''
-(for embedded NICs). The new syntax allows different settings to be
-provided per NIC.
-
-@subsection -redir (since 2.6.0)
-
-The ``-redir [tcp|udp]:hostport:[guestaddr]:guestport'' argument is
-replaced by either
-``-netdev user,id=x,hostfwd=[tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport''
-(for pluggable NICs, accompanied with ``-device ...,netdev=x'') or
-``-nic user,hostfwd=[tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport''
-(for embedded NICs). The new syntax allows different settings to be
-provided per NIC.
-
-@subsection -smb (since 2.6.0)
-
-The ``-smb /some/dir'' argument is replaced by either
-``-netdev user,id=x,smb=/some/dir '' (for pluggable NICs, accompanied
-with ``-device ...,netdev=x''), or ``-nic user,smb=/some/dir''
-(for embedded NICs). The new syntax allows different settings to be
-provided per NIC.
-
@subsection -usbdevice (since 2.10.0)
The ``-usbdevice DEV'' argument is now a synonym for setting
diff --git a/qemu-options.hx b/qemu-options.hx
index 7ca539a..a642ad2 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1813,16 +1813,6 @@ STEXI
@table @option
ETEXI
-HXCOMM Legacy slirp options (now moved to -net user):
-#ifdef CONFIG_SLIRP
-DEF("tftp", HAS_ARG, QEMU_OPTION_tftp, "", QEMU_ARCH_ALL)
-DEF("bootp", HAS_ARG, QEMU_OPTION_bootp, "", QEMU_ARCH_ALL)
-DEF("redir", HAS_ARG, QEMU_OPTION_redir, "", QEMU_ARCH_ALL)
-#ifndef _WIN32
-DEF("smb", HAS_ARG, QEMU_OPTION_smb, "", QEMU_ARCH_ALL)
-#endif
-#endif
-
DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
#ifdef CONFIG_SLIRP
"-netdev user,id=str[,ipv4[=on|off]][,net=addr[/mask]][,host=addr]\n"
@@ -2150,11 +2140,6 @@ qemu-system-i386 -nic 'user,id=n1,guestfwd=tcp:10.0.2.100:1234-cmd:netcat 10.10
@end table
-Note: Legacy stand-alone options -tftp, -bootp, -smb and -redir are still
-processed and applied to -net user. Mixing them with the new configuration
-syntax gives undefined results. Their use for new applications is discouraged
-as they will be removed from future versions.
-
@item -netdev tap,id=@var{id}[,fd=@var{h}][,ifname=@var{name}][,script=@var{file}][,downscript=@var{dfile}][,br=@var{bridge}][,helper=@var{helper}]
Configure a host TAP network backend with ID @var{id}.
diff --git a/vl.c b/vl.c
index cfeee0d..1b8ff85 100644
--- a/vl.c
+++ b/vl.c
@@ -3168,24 +3168,6 @@ int main(int argc, char **argv, char **envp)
}
break;
#endif
-#ifdef CONFIG_SLIRP
- case QEMU_OPTION_tftp:
- error_report("The -tftp option is deprecated. "
- "Please use '-netdev user,tftp=...' instead.");
- legacy_tftp_prefix = optarg;
- break;
- case QEMU_OPTION_bootp:
- error_report("The -bootp option is deprecated. "
- "Please use '-netdev user,bootfile=...' instead.");
- legacy_bootp_filename = optarg;
- break;
- case QEMU_OPTION_redir:
- error_report("The -redir option is deprecated. "
- "Please use '-netdev user,hostfwd=...' instead.");
- if (net_slirp_redir(optarg) < 0)
- exit(1);
- break;
-#endif
case QEMU_OPTION_bt:
add_device_config(DEV_BT, optarg);
break;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL 5/9] tests/libqos: Utilize newer glib spawn check
2018-08-31 8:38 [Qemu-devel] [PULL 0/9] Removal of deprecated options and improvements for qtests Thomas Huth
` (3 preceding siblings ...)
2018-08-31 8:38 ` [Qemu-devel] [PULL 4/9] net: Remove the deprecated -tftp, -bootp, -redir and -smb options Thomas Huth
@ 2018-08-31 8:38 ` Thomas Huth
2018-08-31 8:38 ` [Qemu-devel] [PULL 6/9] tests: add qmp_assert_error_class() Thomas Huth
` (3 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2018-08-31 8:38 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Paolo Bonzini, Laurent Vivier, Eric Blake, Marc-André Lureau
From: Eric Blake <eblake@redhat.com>
During development, I got a 'make check' failure that claimed:
qemu-img returned status code 32512
**
ERROR:tests/libqos/libqos.c:202:mkimg: assertion failed: (!rc)
But 32512 is too big for a normal exit status value, which means we
failed to use WEXITSTATUS() to shift the bits to the desired value
for printing. However, instead of worrying about how to portably
parse g_spawn()'s rc in the proper platform-dependent manner, it's
better to just rely on the fact that we now require glib 2.40 (since
commit e7b3af815) and can therefore use glib's portable checker
instead, where the message under my same condition improves to:
Child process exited with code 127
**
ERROR:tests/libqos/libqos.c:192:mkimg: assertion failed: (ret && !err)
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/libqos/libqos.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
index 013ca68..c514187 100644
--- a/tests/libqos/libqos.c
+++ b/tests/libqos/libqos.c
@@ -185,22 +185,12 @@ void mkimg(const char *file, const char *fmt, unsigned size_mb)
cli = g_strdup_printf("%s create -f %s %s %uM", qemu_img_abs_path,
fmt, file, size_mb);
ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err);
- if (err) {
+ if (err || !g_spawn_check_exit_status(rc, &err)) {
fprintf(stderr, "%s\n", err->message);
g_error_free(err);
}
g_assert(ret && !err);
- /* In glib 2.34, we have g_spawn_check_exit_status. in 2.12, we don't.
- * glib 2.43.91 implementation assumes that any non-zero is an error for
- * windows, but uses extra precautions for Linux. However,
- * 0 is only possible if the program exited normally, so that should be
- * sufficient for our purposes on all platforms, here. */
- if (rc) {
- fprintf(stderr, "qemu-img returned status code %d\n", rc);
- }
- g_assert(!rc);
-
g_free(out);
g_free(out2);
g_free(cli);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL 6/9] tests: add qmp_assert_error_class()
2018-08-31 8:38 [Qemu-devel] [PULL 0/9] Removal of deprecated options and improvements for qtests Thomas Huth
` (4 preceding siblings ...)
2018-08-31 8:38 ` [Qemu-devel] [PULL 5/9] tests/libqos: Utilize newer glib spawn check Thomas Huth
@ 2018-08-31 8:38 ` Thomas Huth
2018-08-31 8:38 ` [Qemu-devel] [PULL 7/9] tests: add qmp/object-add-without-props test Thomas Huth
` (2 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2018-08-31 8:38 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Paolo Bonzini, Laurent Vivier, Eric Blake, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
This helper will simplify a bunch of code checking for QMP errors and
can be shared by various tests. Note that test-qga does check for
error description as well, so don't replace the code there for now.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/drive_del-test.c | 5 +---
tests/libqtest.c | 11 ++++++++
tests/libqtest.h | 9 +++++++
tests/qmp-test.c | 73 ++++++++++++++++----------------------------------
tests/test-qga.c | 9 ++-----
5 files changed, 46 insertions(+), 61 deletions(-)
diff --git a/tests/drive_del-test.c b/tests/drive_del-test.c
index 673c101..4a1a088 100644
--- a/tests/drive_del-test.c
+++ b/tests/drive_del-test.c
@@ -67,7 +67,6 @@ static void test_after_failed_device_add(void)
{
char driver[32];
QDict *response;
- QDict *error;
snprintf(driver, sizeof(driver), "virtio-blk-%s",
qvirtio_get_dev_type());
@@ -83,9 +82,7 @@ static void test_after_failed_device_add(void)
" 'drive': 'drive0'"
"}}", driver);
g_assert(response);
- error = qdict_get_qdict(response, "error");
- g_assert_cmpstr(qdict_get_try_str(error, "class"), ==, "GenericError");
- qobject_unref(response);
+ qmp_assert_error_class(response, "GenericError");
/* Delete the drive */
drive_del();
diff --git a/tests/libqtest.c b/tests/libqtest.c
index d635c5b..2cd5736 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -1194,3 +1194,14 @@ bool qmp_rsp_is_err(QDict *rsp)
qobject_unref(rsp);
return !!error;
}
+
+void qmp_assert_error_class(QDict *rsp, const char *class)
+{
+ QDict *error = qdict_get_qdict(rsp, "error");
+
+ g_assert_cmpstr(qdict_get_try_str(error, "class"), ==, class);
+ g_assert_nonnull(qdict_get_try_str(error, "desc"));
+ g_assert(!qdict_haskey(rsp, "return"));
+
+ qobject_unref(rsp);
+}
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 36d5cae..ed88ff9 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -1004,4 +1004,13 @@ void qtest_qmp_device_del(const char *id);
*/
bool qmp_rsp_is_err(QDict *rsp);
+/**
+ * qmp_assert_error_class:
+ * @rsp: QMP response to check for error
+ * @class: an error class
+ *
+ * Assert the response has the given error class and discard @rsp.
+ */
+void qmp_assert_error_class(QDict *rsp, const char *class);
+
#endif
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 4ae2245..b347228 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -21,15 +21,6 @@
const char common_args[] = "-nodefaults -machine none";
-static const char *get_error_class(QDict *resp)
-{
- QDict *error = qdict_get_qdict(resp, "error");
- const char *desc = qdict_get_try_str(error, "desc");
-
- g_assert(desc);
- return error ? qdict_get_try_str(error, "class") : NULL;
-}
-
static void test_version(QObject *version)
{
Visitor *v;
@@ -42,15 +33,12 @@ static void test_version(QObject *version)
visit_free(v);
}
-static bool recovered(QTestState *qts)
+static void assert_recovered(QTestState *qts)
{
QDict *resp;
- bool ret;
resp = qtest_qmp(qts, "{ 'execute': 'no-such-cmd' }");
- ret = !strcmp(get_error_class(resp), "CommandNotFound");
- qobject_unref(resp);
- return ret;
+ qmp_assert_error_class(resp, "CommandNotFound");
}
static void test_malformed(QTestState *qts)
@@ -60,73 +48,61 @@ static void test_malformed(QTestState *qts)
/* syntax error */
qtest_qmp_send_raw(qts, "{]\n");
resp = qtest_qmp_receive(qts);
- g_assert_cmpstr(get_error_class(resp), ==, "GenericError");
- qobject_unref(resp);
- g_assert(recovered(qts));
+ qmp_assert_error_class(resp, "GenericError");
+ assert_recovered(qts);
/* lexical error: impossible byte outside string */
qtest_qmp_send_raw(qts, "{\xFF");
resp = qtest_qmp_receive(qts);
- g_assert_cmpstr(get_error_class(resp), ==, "GenericError");
- qobject_unref(resp);
- g_assert(recovered(qts));
+ qmp_assert_error_class(resp, "GenericError");
+ assert_recovered(qts);
/* lexical error: funny control character outside string */
qtest_qmp_send_raw(qts, "{\x01");
resp = qtest_qmp_receive(qts);
- g_assert_cmpstr(get_error_class(resp), ==, "GenericError");
- qobject_unref(resp);
- g_assert(recovered(qts));
+ qmp_assert_error_class(resp, "GenericError");
+ assert_recovered(qts);
/* lexical error: impossible byte in string */
qtest_qmp_send_raw(qts, "{'bad \xFF");
resp = qtest_qmp_receive(qts);
- g_assert_cmpstr(get_error_class(resp), ==, "GenericError");
- qobject_unref(resp);
- g_assert(recovered(qts));
+ qmp_assert_error_class(resp, "GenericError");
+ assert_recovered(qts);
/* lexical error: control character in string */
qtest_qmp_send_raw(qts, "{'execute': 'nonexistent', 'id':'\n");
resp = qtest_qmp_receive(qts);
- g_assert_cmpstr(get_error_class(resp), ==, "GenericError");
- qobject_unref(resp);
- g_assert(recovered(qts));
+ qmp_assert_error_class(resp, "GenericError");
+ assert_recovered(qts);
/* lexical error: interpolation */
qtest_qmp_send_raw(qts, "%%p\n");
/* two errors, one for "%", one for "p" */
resp = qtest_qmp_receive(qts);
- g_assert_cmpstr(get_error_class(resp), ==, "GenericError");
- qobject_unref(resp);
+ qmp_assert_error_class(resp, "GenericError");
resp = qtest_qmp_receive(qts);
- g_assert_cmpstr(get_error_class(resp), ==, "GenericError");
- qobject_unref(resp);
- g_assert(recovered(qts));
+ qmp_assert_error_class(resp, "GenericError");
+ assert_recovered(qts);
/* Not even a dictionary */
resp = qtest_qmp(qts, "null");
- g_assert_cmpstr(get_error_class(resp), ==, "GenericError");
- qobject_unref(resp);
+ qmp_assert_error_class(resp, "GenericError");
/* No "execute" key */
resp = qtest_qmp(qts, "{}");
- g_assert_cmpstr(get_error_class(resp), ==, "GenericError");
- qobject_unref(resp);
+ qmp_assert_error_class(resp, "GenericError");
/* "execute" isn't a string */
resp = qtest_qmp(qts, "{ 'execute': true }");
- g_assert_cmpstr(get_error_class(resp), ==, "GenericError");
- qobject_unref(resp);
+ qmp_assert_error_class(resp, "GenericError");
/* "arguments" isn't a dictionary */
resp = qtest_qmp(qts, "{ 'execute': 'no-such-cmd', 'arguments': [] }");
- g_assert_cmpstr(get_error_class(resp), ==, "GenericError");
- qobject_unref(resp);
+ qmp_assert_error_class(resp, "GenericError");
/* extra key */
resp = qtest_qmp(qts, "{ 'execute': 'no-such-cmd', 'extra': true }");
- g_assert_cmpstr(get_error_class(resp), ==, "GenericError");
- qobject_unref(resp);
+ qmp_assert_error_class(resp, "GenericError");
}
static void test_qmp_protocol(void)
@@ -148,8 +124,7 @@ static void test_qmp_protocol(void)
/* Test valid command before handshake */
resp = qtest_qmp(qts, "{ 'execute': 'query-version' }");
- g_assert_cmpstr(get_error_class(resp), ==, "CommandNotFound");
- qobject_unref(resp);
+ qmp_assert_error_class(resp, "CommandNotFound");
/* Test malformed commands before handshake */
test_malformed(qts);
@@ -162,8 +137,7 @@ static void test_qmp_protocol(void)
/* Test repeated handshake */
resp = qtest_qmp(qts, "{ 'execute': 'qmp_capabilities' }");
- g_assert_cmpstr(get_error_class(resp), ==, "CommandNotFound");
- qobject_unref(resp);
+ qmp_assert_error_class(resp, "CommandNotFound");
/* Test valid command */
resp = qtest_qmp(qts, "{ 'execute': 'query-version' }");
@@ -182,9 +156,8 @@ static void test_qmp_protocol(void)
/* Test command failure with 'id' */
resp = qtest_qmp(qts, "{ 'execute': 'human-monitor-command', 'id': 2 }");
- g_assert_cmpstr(get_error_class(resp), ==, "GenericError");
g_assert_cmpint(qdict_get_int(resp, "id"), ==, 2);
- qobject_unref(resp);
+ qmp_assert_error_class(resp, "GenericError");
qtest_quit(qts);
}
diff --git a/tests/test-qga.c b/tests/test-qga.c
index f69cdf6..3d63774 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -244,17 +244,12 @@ static void test_qga_invalid_id(gconstpointer fix)
static void test_qga_invalid_oob(gconstpointer fix)
{
const TestFixture *fixture = fix;
- QDict *ret, *error;
- const char *class;
+ QDict *ret;
ret = qmp_fd(fixture->fd, "{'exec-oob': 'guest-ping'}");
g_assert_nonnull(ret);
- error = qdict_get_qdict(ret, "error");
- class = qdict_get_try_str(error, "class");
- g_assert_cmpstr(class, ==, "GenericError");
-
- qobject_unref(ret);
+ qmp_assert_error_class(ret, "GenericError");
}
static void test_qga_invalid_args(gconstpointer fix)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL 7/9] tests: add qmp/object-add-without-props test
2018-08-31 8:38 [Qemu-devel] [PULL 0/9] Removal of deprecated options and improvements for qtests Thomas Huth
` (5 preceding siblings ...)
2018-08-31 8:38 ` [Qemu-devel] [PULL 6/9] tests: add qmp_assert_error_class() Thomas Huth
@ 2018-08-31 8:38 ` Thomas Huth
2018-08-31 8:38 ` [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test Thomas Huth
2018-08-31 8:38 ` [Qemu-devel] [PULL 9/9] tests: add a qmp success-response test Thomas Huth
8 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2018-08-31 8:38 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Paolo Bonzini, Laurent Vivier, Eric Blake, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
test_object_add_without_props() tests a bug in qmp_object_add() we
fixed in commit e64c75a975. Sadly, we don't have systematic
object-add tests. This lone test can go into qmp-cmd-test for want of
a better home.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/qmp-cmd-test.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/tests/qmp-cmd-test.c b/tests/qmp-cmd-test.c
index c5b70df..d12cac5 100644
--- a/tests/qmp-cmd-test.c
+++ b/tests/qmp-cmd-test.c
@@ -197,6 +197,19 @@ static void add_query_tests(QmpSchema *schema)
}
}
+static void test_object_add_without_props(void)
+{
+ QTestState *qts;
+ QDict *resp;
+
+ qts = qtest_init(common_args);
+ resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
+ " {'qom-type': 'memory-backend-ram', 'id': 'ram1' } }");
+ g_assert_nonnull(resp);
+ qmp_assert_error_class(resp, "GenericError");
+ qtest_quit(qts);
+}
+
int main(int argc, char *argv[])
{
QmpSchema schema;
@@ -206,6 +219,11 @@ int main(int argc, char *argv[])
qmp_schema_init(&schema);
add_query_tests(&schema);
+
+ qtest_add_func("qmp/object-add-without-props",
+ test_object_add_without_props);
+ /* TODO: add coverage of generic object-add failure modes */
+
ret = g_test_run();
qmp_schema_cleanup(&schema);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test
2018-08-31 8:38 [Qemu-devel] [PULL 0/9] Removal of deprecated options and improvements for qtests Thomas Huth
` (6 preceding siblings ...)
2018-08-31 8:38 ` [Qemu-devel] [PULL 7/9] tests: add qmp/object-add-without-props test Thomas Huth
@ 2018-08-31 8:38 ` Thomas Huth
2018-08-31 12:04 ` Markus Armbruster
2018-08-31 8:38 ` [Qemu-devel] [PULL 9/9] tests: add a qmp success-response test Thomas Huth
8 siblings, 1 reply; 19+ messages in thread
From: Thomas Huth @ 2018-08-31 8:38 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Paolo Bonzini, Laurent Vivier, Eric Blake, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
test_qom_set_without_value() is about a bug in infrastructure used by
the QMP core, fixed in commit c489780203. We covered the bug in
infrastructure unit tests (commit bce3035a44). I wrote that test
earlier, to cover QMP level as well, the test could go into qmp-test.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/qmp-test.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index b347228..2b923f0 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -321,6 +321,19 @@ static void test_qmp_preconfig(void)
qtest_quit(qs);
}
+static void test_qom_set_without_value(void)
+{
+ QTestState *qts;
+ QDict *resp;
+
+ qts = qtest_init(common_args);
+ resp = qtest_qmp(qts, "{'execute': 'qom-set', 'arguments':"
+ " { 'path': '/machine', 'property': 'rtc-time' } }");
+ g_assert_nonnull(resp);
+ qmp_assert_error_class(resp, "GenericError");
+ qtest_quit(qts);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -328,6 +341,7 @@ int main(int argc, char *argv[])
qtest_add_func("qmp/protocol", test_qmp_protocol);
qtest_add_func("qmp/oob", test_qmp_oob);
qtest_add_func("qmp/preconfig", test_qmp_preconfig);
+ qtest_add_func("qmp/qom-set-without-value", test_qom_set_without_value);
return g_test_run();
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test
2018-08-31 8:38 ` [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test Thomas Huth
@ 2018-08-31 12:04 ` Markus Armbruster
2018-08-31 13:18 ` Thomas Huth
0 siblings, 1 reply; 19+ messages in thread
From: Markus Armbruster @ 2018-08-31 12:04 UTC (permalink / raw)
To: Thomas Huth
Cc: Peter Maydell, qemu-devel, Laurent Vivier, Paolo Bonzini,
Marc-André Lureau
Thomas Huth <thuth@redhat.com> writes:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> test_qom_set_without_value() is about a bug in infrastructure used by
> the QMP core, fixed in commit c489780203. We covered the bug in
> infrastructure unit tests (commit bce3035a44). I wrote that test
> earlier, to cover QMP level as well, the test could go into qmp-test.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> tests/qmp-test.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
> index b347228..2b923f0 100644
> --- a/tests/qmp-test.c
> +++ b/tests/qmp-test.c
> @@ -321,6 +321,19 @@ static void test_qmp_preconfig(void)
> qtest_quit(qs);
> }
>
> +static void test_qom_set_without_value(void)
> +{
> + QTestState *qts;
> + QDict *resp;
> +
> + qts = qtest_init(common_args);
> + resp = qtest_qmp(qts, "{'execute': 'qom-set', 'arguments':"
> + " { 'path': '/machine', 'property': 'rtc-time' } }");
> + g_assert_nonnull(resp);
> + qmp_assert_error_class(resp, "GenericError");
> + qtest_quit(qts);
> +}
> +
> int main(int argc, char *argv[])
> {
> g_test_init(&argc, &argv, NULL);
> @@ -328,6 +341,7 @@ int main(int argc, char *argv[])
> qtest_add_func("qmp/protocol", test_qmp_protocol);
> qtest_add_func("qmp/oob", test_qmp_oob);
> qtest_add_func("qmp/preconfig", test_qmp_preconfig);
> + qtest_add_func("qmp/qom-set-without-value", test_qom_set_without_value);
>
> return g_test_run();
> }
I'm afraid you missed my objection to naming:
Message-ID: <8736uvujxx.fsf@dusky.pond.sub.org>
https://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg06368.html
If you could work that into PULL v2, I'd be obliged. If not, I'll have
to address it in a follow-up patch.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test
2018-08-31 12:04 ` Markus Armbruster
@ 2018-08-31 13:18 ` Thomas Huth
2018-08-31 13:24 ` Marc-André Lureau
0 siblings, 1 reply; 19+ messages in thread
From: Thomas Huth @ 2018-08-31 13:18 UTC (permalink / raw)
To: Markus Armbruster
Cc: Peter Maydell, qemu-devel, Laurent Vivier, Paolo Bonzini,
Marc-André Lureau
On 2018-08-31 14:04, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> writes:
>
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> test_qom_set_without_value() is about a bug in infrastructure used by
>> the QMP core, fixed in commit c489780203. We covered the bug in
>> infrastructure unit tests (commit bce3035a44). I wrote that test
>> earlier, to cover QMP level as well, the test could go into qmp-test.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> tests/qmp-test.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
>> index b347228..2b923f0 100644
>> --- a/tests/qmp-test.c
>> +++ b/tests/qmp-test.c
>> @@ -321,6 +321,19 @@ static void test_qmp_preconfig(void)
>> qtest_quit(qs);
>> }
>>
>> +static void test_qom_set_without_value(void)
>> +{
>> + QTestState *qts;
>> + QDict *resp;
>> +
>> + qts = qtest_init(common_args);
>> + resp = qtest_qmp(qts, "{'execute': 'qom-set', 'arguments':"
>> + " { 'path': '/machine', 'property': 'rtc-time' } }");
>> + g_assert_nonnull(resp);
>> + qmp_assert_error_class(resp, "GenericError");
>> + qtest_quit(qts);
>> +}
>> +
>> int main(int argc, char *argv[])
>> {
>> g_test_init(&argc, &argv, NULL);
>> @@ -328,6 +341,7 @@ int main(int argc, char *argv[])
>> qtest_add_func("qmp/protocol", test_qmp_protocol);
>> qtest_add_func("qmp/oob", test_qmp_oob);
>> qtest_add_func("qmp/preconfig", test_qmp_preconfig);
>> + qtest_add_func("qmp/qom-set-without-value", test_qom_set_without_value);
>>
>> return g_test_run();
>> }
>
> I'm afraid you missed my objection to naming:
> Message-ID: <8736uvujxx.fsf@dusky.pond.sub.org>
> https://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg06368.html
Sorry about that, I was not on CC: for that series. I used the patches
from v5 where Marc-André put me on CC:.
> If you could work that into PULL v2, I'd be obliged. If not, I'll have
> to address it in a follow-up patch.
IMHO the naming is not that bad ... OTOH, I think Peter might already be
away? ... so we've got plenty of time to sort this out anyway.
Marc-André, could you send a new version of the patch?
Thomas
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test
2018-08-31 13:18 ` Thomas Huth
@ 2018-08-31 13:24 ` Marc-André Lureau
2018-08-31 13:40 ` Thomas Huth
0 siblings, 1 reply; 19+ messages in thread
From: Marc-André Lureau @ 2018-08-31 13:24 UTC (permalink / raw)
To: Thomas Huth
Cc: Markus Armbruster, Laurent Vivier, Peter Maydell, QEMU, Paolo Bonzini
Hi
On Fri, Aug 31, 2018 at 3:18 PM Thomas Huth <thuth@redhat.com> wrote:
>
> On 2018-08-31 14:04, Markus Armbruster wrote:
> > Thomas Huth <thuth@redhat.com> writes:
> >
> >> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>
> >> test_qom_set_without_value() is about a bug in infrastructure used by
> >> the QMP core, fixed in commit c489780203. We covered the bug in
> >> infrastructure unit tests (commit bce3035a44). I wrote that test
> >> earlier, to cover QMP level as well, the test could go into qmp-test.
> >>
> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >> Reviewed-by: Thomas Huth <thuth@redhat.com>
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >> ---
> >> tests/qmp-test.c | 14 ++++++++++++++
> >> 1 file changed, 14 insertions(+)
> >>
> >> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
> >> index b347228..2b923f0 100644
> >> --- a/tests/qmp-test.c
> >> +++ b/tests/qmp-test.c
> >> @@ -321,6 +321,19 @@ static void test_qmp_preconfig(void)
> >> qtest_quit(qs);
> >> }
> >>
> >> +static void test_qom_set_without_value(void)
> >> +{
> >> + QTestState *qts;
> >> + QDict *resp;
> >> +
> >> + qts = qtest_init(common_args);
> >> + resp = qtest_qmp(qts, "{'execute': 'qom-set', 'arguments':"
> >> + " { 'path': '/machine', 'property': 'rtc-time' } }");
> >> + g_assert_nonnull(resp);
> >> + qmp_assert_error_class(resp, "GenericError");
> >> + qtest_quit(qts);
> >> +}
> >> +
> >> int main(int argc, char *argv[])
> >> {
> >> g_test_init(&argc, &argv, NULL);
> >> @@ -328,6 +341,7 @@ int main(int argc, char *argv[])
> >> qtest_add_func("qmp/protocol", test_qmp_protocol);
> >> qtest_add_func("qmp/oob", test_qmp_oob);
> >> qtest_add_func("qmp/preconfig", test_qmp_preconfig);
> >> + qtest_add_func("qmp/qom-set-without-value", test_qom_set_without_value);
> >>
> >> return g_test_run();
> >> }
> >
> > I'm afraid you missed my objection to naming:
> > Message-ID: <8736uvujxx.fsf@dusky.pond.sub.org>
> > https://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg06368.html
>
> Sorry about that, I was not on CC: for that series. I used the patches
> from v5 where Marc-André put me on CC:.
>
> > If you could work that into PULL v2, I'd be obliged. If not, I'll have
> > to address it in a follow-up patch.
>
> IMHO the naming is not that bad ... OTOH, I think Peter might already be
> away? ... so we've got plenty of time to sort this out anyway.
> Marc-André, could you send a new version of the patch?
Tbh, I don't care so much about the naming of the test, but (for once)
I respectfully don't think Markus suggestion is better.
The function checks "qom-set" without 'value' argument:
"qom-set-without-value", no brainer..
Naming it "invalid-arg" is so generic that I wouldn't be able what it does.
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test
2018-08-31 13:24 ` Marc-André Lureau
@ 2018-08-31 13:40 ` Thomas Huth
2018-08-31 14:35 ` Markus Armbruster
0 siblings, 1 reply; 19+ messages in thread
From: Thomas Huth @ 2018-08-31 13:40 UTC (permalink / raw)
To: Marc-André Lureau
Cc: Markus Armbruster, Laurent Vivier, Peter Maydell, QEMU, Paolo Bonzini
On 2018-08-31 15:24, Marc-André Lureau wrote:
> Hi
> On Fri, Aug 31, 2018 at 3:18 PM Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 2018-08-31 14:04, Markus Armbruster wrote:
>>> Thomas Huth <thuth@redhat.com> writes:
>>>
>>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>
>>>> test_qom_set_without_value() is about a bug in infrastructure used by
>>>> the QMP core, fixed in commit c489780203. We covered the bug in
>>>> infrastructure unit tests (commit bce3035a44). I wrote that test
>>>> earlier, to cover QMP level as well, the test could go into qmp-test.
>>>>
>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>> tests/qmp-test.c | 14 ++++++++++++++
>>>> 1 file changed, 14 insertions(+)
>>>>
>>>> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
>>>> index b347228..2b923f0 100644
>>>> --- a/tests/qmp-test.c
>>>> +++ b/tests/qmp-test.c
>>>> @@ -321,6 +321,19 @@ static void test_qmp_preconfig(void)
>>>> qtest_quit(qs);
>>>> }
>>>>
>>>> +static void test_qom_set_without_value(void)
>>>> +{
>>>> + QTestState *qts;
>>>> + QDict *resp;
>>>> +
>>>> + qts = qtest_init(common_args);
>>>> + resp = qtest_qmp(qts, "{'execute': 'qom-set', 'arguments':"
>>>> + " { 'path': '/machine', 'property': 'rtc-time' } }");
>>>> + g_assert_nonnull(resp);
>>>> + qmp_assert_error_class(resp, "GenericError");
>>>> + qtest_quit(qts);
>>>> +}
>>>> +
>>>> int main(int argc, char *argv[])
>>>> {
>>>> g_test_init(&argc, &argv, NULL);
>>>> @@ -328,6 +341,7 @@ int main(int argc, char *argv[])
>>>> qtest_add_func("qmp/protocol", test_qmp_protocol);
>>>> qtest_add_func("qmp/oob", test_qmp_oob);
>>>> qtest_add_func("qmp/preconfig", test_qmp_preconfig);
>>>> + qtest_add_func("qmp/qom-set-without-value", test_qom_set_without_value);
>>>>
>>>> return g_test_run();
>>>> }
>>>
>>> I'm afraid you missed my objection to naming:
>>> Message-ID: <8736uvujxx.fsf@dusky.pond.sub.org>
>>> https://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg06368.html
>>
>> Sorry about that, I was not on CC: for that series. I used the patches
>> from v5 where Marc-André put me on CC:.
>>
>>> If you could work that into PULL v2, I'd be obliged. If not, I'll have
>>> to address it in a follow-up patch.
>>
>> IMHO the naming is not that bad ... OTOH, I think Peter might already be
>> away? ... so we've got plenty of time to sort this out anyway.
>> Marc-André, could you send a new version of the patch?
>
> Tbh, I don't care so much about the naming of the test, but (for once)
> I respectfully don't think Markus suggestion is better.
>
> The function checks "qom-set" without 'value' argument:
> "qom-set-without-value", no brainer..
>
> Naming it "invalid-arg" is so generic that I wouldn't be able what it does.
Ok, then let's keep it this way. As I said, IMHO the current naming is
not really bad, and I also don't have any suggestions for a perfect name
right now.
Thomas
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test
2018-08-31 13:40 ` Thomas Huth
@ 2018-08-31 14:35 ` Markus Armbruster
2018-09-03 4:45 ` Thomas Huth
2018-10-09 16:41 ` Markus Armbruster
0 siblings, 2 replies; 19+ messages in thread
From: Markus Armbruster @ 2018-08-31 14:35 UTC (permalink / raw)
To: Thomas Huth
Cc: Marc-André Lureau, Laurent Vivier, Peter Maydell,
Paolo Bonzini, Markus Armbruster, QEMU
Thomas Huth <thuth@redhat.com> writes:
> On 2018-08-31 15:24, Marc-André Lureau wrote:
>> Hi
>> On Fri, Aug 31, 2018 at 3:18 PM Thomas Huth <thuth@redhat.com> wrote:
>>>
>>> On 2018-08-31 14:04, Markus Armbruster wrote:
>>>> Thomas Huth <thuth@redhat.com> writes:
>>>>
>>>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>
>>>>> test_qom_set_without_value() is about a bug in infrastructure used by
>>>>> the QMP core, fixed in commit c489780203. We covered the bug in
>>>>> infrastructure unit tests (commit bce3035a44). I wrote that test
>>>>> earlier, to cover QMP level as well, the test could go into qmp-test.
>>>>>
>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>> ---
>>>>> tests/qmp-test.c | 14 ++++++++++++++
>>>>> 1 file changed, 14 insertions(+)
>>>>>
>>>>> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
>>>>> index b347228..2b923f0 100644
>>>>> --- a/tests/qmp-test.c
>>>>> +++ b/tests/qmp-test.c
>>>>> @@ -321,6 +321,19 @@ static void test_qmp_preconfig(void)
>>>>> qtest_quit(qs);
>>>>> }
>>>>>
>>>>> +static void test_qom_set_without_value(void)
>>>>> +{
>>>>> + QTestState *qts;
>>>>> + QDict *resp;
>>>>> +
>>>>> + qts = qtest_init(common_args);
>>>>> + resp = qtest_qmp(qts, "{'execute': 'qom-set', 'arguments':"
>>>>> + " { 'path': '/machine', 'property': 'rtc-time' } }");
>>>>> + g_assert_nonnull(resp);
>>>>> + qmp_assert_error_class(resp, "GenericError");
>>>>> + qtest_quit(qts);
>>>>> +}
>>>>> +
>>>>> int main(int argc, char *argv[])
>>>>> {
>>>>> g_test_init(&argc, &argv, NULL);
>>>>> @@ -328,6 +341,7 @@ int main(int argc, char *argv[])
>>>>> qtest_add_func("qmp/protocol", test_qmp_protocol);
>>>>> qtest_add_func("qmp/oob", test_qmp_oob);
>>>>> qtest_add_func("qmp/preconfig", test_qmp_preconfig);
>>>>> + qtest_add_func("qmp/qom-set-without-value", test_qom_set_without_value);
>>>>>
>>>>> return g_test_run();
>>>>> }
>>>>
>>>> I'm afraid you missed my objection to naming:
>>>> Message-ID: <8736uvujxx.fsf@dusky.pond.sub.org>
>>>> https://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg06368.html
>>>
>>> Sorry about that, I was not on CC: for that series. I used the patches
>>> from v5 where Marc-André put me on CC:.
>>>
>>>> If you could work that into PULL v2, I'd be obliged. If not, I'll have
>>>> to address it in a follow-up patch.
>>>
>>> IMHO the naming is not that bad ... OTOH, I think Peter might already be
>>> away? ... so we've got plenty of time to sort this out anyway.
>>> Marc-André, could you send a new version of the patch?
>>
>> Tbh, I don't care so much about the naming of the test, but (for once)
>> I respectfully don't think Markus suggestion is better.
>>
>> The function checks "qom-set" without 'value' argument:
>> "qom-set-without-value", no brainer..
Nope, that's not what it tests. It tests the visitor, the marshalling
code generator, and the QMP core handle a certain kind of invalid
arguments correctly. It does not test qom-set. I explained all that
already.
>> Naming it "invalid-arg" is so generic that I wouldn't be able what it does.
I can accept "missing-any" or "missing-any-arg". I object to any name
involving qom-set, because the test is not about qom-set at all.
If it was, then putting it in qmp-test.c would be wrong.
> Ok, then let's keep it this way. As I said, IMHO the current naming is
> not really bad, and I also don't have any suggestions for a perfect name
> right now.
We don't need a perfect name. We need one that's not actively
misleading.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test
2018-08-31 14:35 ` Markus Armbruster
@ 2018-09-03 4:45 ` Thomas Huth
2018-10-09 16:41 ` Markus Armbruster
1 sibling, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2018-09-03 4:45 UTC (permalink / raw)
To: Markus Armbruster
Cc: Marc-André Lureau, Laurent Vivier, Peter Maydell,
Paolo Bonzini, QEMU
On 2018-08-31 16:35, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> writes:
>
>> On 2018-08-31 15:24, Marc-André Lureau wrote:
>>> Hi
>>> On Fri, Aug 31, 2018 at 3:18 PM Thomas Huth <thuth@redhat.com> wrote:
>>>>
>>>> On 2018-08-31 14:04, Markus Armbruster wrote:
>>>>> Thomas Huth <thuth@redhat.com> writes:
>>>>>
>>>>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>>
>>>>>> test_qom_set_without_value() is about a bug in infrastructure used by
>>>>>> the QMP core, fixed in commit c489780203. We covered the bug in
>>>>>> infrastructure unit tests (commit bce3035a44). I wrote that test
>>>>>> earlier, to cover QMP level as well, the test could go into qmp-test.
>>>>>>
>>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>>> ---
>>>>>> tests/qmp-test.c | 14 ++++++++++++++
>>>>>> 1 file changed, 14 insertions(+)
>>>>>>
>>>>>> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
>>>>>> index b347228..2b923f0 100644
>>>>>> --- a/tests/qmp-test.c
>>>>>> +++ b/tests/qmp-test.c
>>>>>> @@ -321,6 +321,19 @@ static void test_qmp_preconfig(void)
>>>>>> qtest_quit(qs);
>>>>>> }
>>>>>>
>>>>>> +static void test_qom_set_without_value(void)
>>>>>> +{
>>>>>> + QTestState *qts;
>>>>>> + QDict *resp;
>>>>>> +
>>>>>> + qts = qtest_init(common_args);
>>>>>> + resp = qtest_qmp(qts, "{'execute': 'qom-set', 'arguments':"
>>>>>> + " { 'path': '/machine', 'property': 'rtc-time' } }");
>>>>>> + g_assert_nonnull(resp);
>>>>>> + qmp_assert_error_class(resp, "GenericError");
>>>>>> + qtest_quit(qts);
>>>>>> +}
>>>>>> +
>>>>>> int main(int argc, char *argv[])
>>>>>> {
>>>>>> g_test_init(&argc, &argv, NULL);
>>>>>> @@ -328,6 +341,7 @@ int main(int argc, char *argv[])
>>>>>> qtest_add_func("qmp/protocol", test_qmp_protocol);
>>>>>> qtest_add_func("qmp/oob", test_qmp_oob);
>>>>>> qtest_add_func("qmp/preconfig", test_qmp_preconfig);
>>>>>> + qtest_add_func("qmp/qom-set-without-value", test_qom_set_without_value);
>>>>>>
>>>>>> return g_test_run();
>>>>>> }
>>>>>
>>>>> I'm afraid you missed my objection to naming:
>>>>> Message-ID: <8736uvujxx.fsf@dusky.pond.sub.org>
>>>>> https://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg06368.html
>>>>
>>>> Sorry about that, I was not on CC: for that series. I used the patches
>>>> from v5 where Marc-André put me on CC:.
>>>>
>>>>> If you could work that into PULL v2, I'd be obliged. If not, I'll have
>>>>> to address it in a follow-up patch.
>>>>
>>>> IMHO the naming is not that bad ... OTOH, I think Peter might already be
>>>> away? ... so we've got plenty of time to sort this out anyway.
>>>> Marc-André, could you send a new version of the patch?
>>>
>>> Tbh, I don't care so much about the naming of the test, but (for once)
>>> I respectfully don't think Markus suggestion is better.
>>>
>>> The function checks "qom-set" without 'value' argument:
>>> "qom-set-without-value", no brainer..
>
> Nope, that's not what it tests. It tests the visitor, the marshalling
> code generator, and the QMP core handle a certain kind of invalid
> arguments correctly. It does not test qom-set. I explained all that
> already.
>
>>> Naming it "invalid-arg" is so generic that I wouldn't be able what it does.
>
> I can accept "missing-any" or "missing-any-arg". I object to any name
> involving qom-set, because the test is not about qom-set at all.
>
> If it was, then putting it in qmp-test.c would be wrong.
>
>> Ok, then let's keep it this way. As I said, IMHO the current naming is
>> not really bad, and I also don't have any suggestions for a perfect name
>> right now.
>
> We don't need a perfect name. We need one that's not actively
> misleading.
Ok, then let's cancel this PULL request. I'll send a new one after the
"vacation freeze" (i.e. in three weeks), that should be enough time for
both of you to come to an agreement about the best naming here.
Thomas
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test
2018-08-31 14:35 ` Markus Armbruster
2018-09-03 4:45 ` Thomas Huth
@ 2018-10-09 16:41 ` Markus Armbruster
2018-10-29 6:49 ` Marc-André Lureau
1 sibling, 1 reply; 19+ messages in thread
From: Markus Armbruster @ 2018-10-09 16:41 UTC (permalink / raw)
To: Marc-André Lureau
Cc: Thomas Huth, Laurent Vivier, Peter Maydell, Paolo Bonzini, QEMU
Markus Armbruster <armbru@redhat.com> writes:
> Thomas Huth <thuth@redhat.com> writes:
>
>> On 2018-08-31 15:24, Marc-André Lureau wrote:
>>> Hi
>>> On Fri, Aug 31, 2018 at 3:18 PM Thomas Huth <thuth@redhat.com> wrote:
>>>>
>>>> On 2018-08-31 14:04, Markus Armbruster wrote:
>>>>> Thomas Huth <thuth@redhat.com> writes:
>>>>>
>>>>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>>
>>>>>> test_qom_set_without_value() is about a bug in infrastructure used by
>>>>>> the QMP core, fixed in commit c489780203. We covered the bug in
>>>>>> infrastructure unit tests (commit bce3035a44). I wrote that test
>>>>>> earlier, to cover QMP level as well, the test could go into qmp-test.
>>>>>>
>>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>>> ---
>>>>>> tests/qmp-test.c | 14 ++++++++++++++
>>>>>> 1 file changed, 14 insertions(+)
>>>>>>
>>>>>> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
>>>>>> index b347228..2b923f0 100644
>>>>>> --- a/tests/qmp-test.c
>>>>>> +++ b/tests/qmp-test.c
>>>>>> @@ -321,6 +321,19 @@ static void test_qmp_preconfig(void)
>>>>>> qtest_quit(qs);
>>>>>> }
>>>>>>
>>>>>> +static void test_qom_set_without_value(void)
>>>>>> +{
>>>>>> + QTestState *qts;
>>>>>> + QDict *resp;
>>>>>> +
>>>>>> + qts = qtest_init(common_args);
>>>>>> + resp = qtest_qmp(qts, "{'execute': 'qom-set', 'arguments':"
>>>>>> + " { 'path': '/machine', 'property': 'rtc-time' } }");
>>>>>> + g_assert_nonnull(resp);
>>>>>> + qmp_assert_error_class(resp, "GenericError");
>>>>>> + qtest_quit(qts);
>>>>>> +}
>>>>>> +
>>>>>> int main(int argc, char *argv[])
>>>>>> {
>>>>>> g_test_init(&argc, &argv, NULL);
>>>>>> @@ -328,6 +341,7 @@ int main(int argc, char *argv[])
>>>>>> qtest_add_func("qmp/protocol", test_qmp_protocol);
>>>>>> qtest_add_func("qmp/oob", test_qmp_oob);
>>>>>> qtest_add_func("qmp/preconfig", test_qmp_preconfig);
>>>>>> + qtest_add_func("qmp/qom-set-without-value", test_qom_set_without_value);
>>>>>>
>>>>>> return g_test_run();
>>>>>> }
>>>>>
>>>>> I'm afraid you missed my objection to naming:
>>>>> Message-ID: <8736uvujxx.fsf@dusky.pond.sub.org>
>>>>> https://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg06368.html
>>>>
>>>> Sorry about that, I was not on CC: for that series. I used the patches
>>>> from v5 where Marc-André put me on CC:.
>>>>
>>>>> If you could work that into PULL v2, I'd be obliged. If not, I'll have
>>>>> to address it in a follow-up patch.
>>>>
>>>> IMHO the naming is not that bad ... OTOH, I think Peter might already be
>>>> away? ... so we've got plenty of time to sort this out anyway.
>>>> Marc-André, could you send a new version of the patch?
>>>
>>> Tbh, I don't care so much about the naming of the test, but (for once)
>>> I respectfully don't think Markus suggestion is better.
>>>
>>> The function checks "qom-set" without 'value' argument:
>>> "qom-set-without-value", no brainer..
>
> Nope, that's not what it tests. It tests the visitor, the marshalling
> code generator, and the QMP core handle a certain kind of invalid
> arguments correctly. It does not test qom-set. I explained all that
> already.
>
>>> Naming it "invalid-arg" is so generic that I wouldn't be able what it does.
>
> I can accept "missing-any" or "missing-any-arg". I object to any name
> involving qom-set, because the test is not about qom-set at all.
>
> If it was, then putting it in qmp-test.c would be wrong.
>
>> Ok, then let's keep it this way. As I said, IMHO the current naming is
>> not really bad, and I also don't have any suggestions for a perfect name
>> right now.
>
> We don't need a perfect name. We need one that's not actively
> misleading.
Marc-André, would "qmp/missing-any-arg" and test_missing_any_arg() work
for you?
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test
2018-10-09 16:41 ` Markus Armbruster
@ 2018-10-29 6:49 ` Marc-André Lureau
2018-10-29 8:38 ` Markus Armbruster
0 siblings, 1 reply; 19+ messages in thread
From: Marc-André Lureau @ 2018-10-29 6:49 UTC (permalink / raw)
To: Markus Armbruster
Cc: Thomas Huth, Laurent Vivier, Peter Maydell, Paolo Bonzini, QEMU
Hi
On Tue, Oct 9, 2018 at 8:41 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> Markus Armbruster <armbru@redhat.com> writes:
>
> > Thomas Huth <thuth@redhat.com> writes:
> >
> >> On 2018-08-31 15:24, Marc-André Lureau wrote:
> >>> Hi
> >>> On Fri, Aug 31, 2018 at 3:18 PM Thomas Huth <thuth@redhat.com> wrote:
> >>>>
> >>>> On 2018-08-31 14:04, Markus Armbruster wrote:
> >>>>> Thomas Huth <thuth@redhat.com> writes:
> >>>>>
> >>>>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>>>>>
> >>>>>> test_qom_set_without_value() is about a bug in infrastructure used by
> >>>>>> the QMP core, fixed in commit c489780203. We covered the bug in
> >>>>>> infrastructure unit tests (commit bce3035a44). I wrote that test
> >>>>>> earlier, to cover QMP level as well, the test could go into qmp-test.
> >>>>>>
> >>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>>>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
> >>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >>>>>> ---
> >>>>>> tests/qmp-test.c | 14 ++++++++++++++
> >>>>>> 1 file changed, 14 insertions(+)
> >>>>>>
> >>>>>> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
> >>>>>> index b347228..2b923f0 100644
> >>>>>> --- a/tests/qmp-test.c
> >>>>>> +++ b/tests/qmp-test.c
> >>>>>> @@ -321,6 +321,19 @@ static void test_qmp_preconfig(void)
> >>>>>> qtest_quit(qs);
> >>>>>> }
> >>>>>>
> >>>>>> +static void test_qom_set_without_value(void)
> >>>>>> +{
> >>>>>> + QTestState *qts;
> >>>>>> + QDict *resp;
> >>>>>> +
> >>>>>> + qts = qtest_init(common_args);
> >>>>>> + resp = qtest_qmp(qts, "{'execute': 'qom-set', 'arguments':"
> >>>>>> + " { 'path': '/machine', 'property': 'rtc-time' } }");
> >>>>>> + g_assert_nonnull(resp);
> >>>>>> + qmp_assert_error_class(resp, "GenericError");
> >>>>>> + qtest_quit(qts);
> >>>>>> +}
> >>>>>> +
> >>>>>> int main(int argc, char *argv[])
> >>>>>> {
> >>>>>> g_test_init(&argc, &argv, NULL);
> >>>>>> @@ -328,6 +341,7 @@ int main(int argc, char *argv[])
> >>>>>> qtest_add_func("qmp/protocol", test_qmp_protocol);
> >>>>>> qtest_add_func("qmp/oob", test_qmp_oob);
> >>>>>> qtest_add_func("qmp/preconfig", test_qmp_preconfig);
> >>>>>> + qtest_add_func("qmp/qom-set-without-value", test_qom_set_without_value);
> >>>>>>
> >>>>>> return g_test_run();
> >>>>>> }
> >>>>>
> >>>>> I'm afraid you missed my objection to naming:
> >>>>> Message-ID: <8736uvujxx.fsf@dusky.pond.sub.org>
> >>>>> https://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg06368.html
> >>>>
> >>>> Sorry about that, I was not on CC: for that series. I used the patches
> >>>> from v5 where Marc-André put me on CC:.
> >>>>
> >>>>> If you could work that into PULL v2, I'd be obliged. If not, I'll have
> >>>>> to address it in a follow-up patch.
> >>>>
> >>>> IMHO the naming is not that bad ... OTOH, I think Peter might already be
> >>>> away? ... so we've got plenty of time to sort this out anyway.
> >>>> Marc-André, could you send a new version of the patch?
> >>>
> >>> Tbh, I don't care so much about the naming of the test, but (for once)
> >>> I respectfully don't think Markus suggestion is better.
> >>>
> >>> The function checks "qom-set" without 'value' argument:
> >>> "qom-set-without-value", no brainer..
> >
> > Nope, that's not what it tests. It tests the visitor, the marshalling
> > code generator, and the QMP core handle a certain kind of invalid
> > arguments correctly. It does not test qom-set. I explained all that
> > already.
> >
> >>> Naming it "invalid-arg" is so generic that I wouldn't be able what it does.
> >
> > I can accept "missing-any" or "missing-any-arg". I object to any name
> > involving qom-set, because the test is not about qom-set at all.
> >
> > If it was, then putting it in qmp-test.c would be wrong.
> >
> >> Ok, then let's keep it this way. As I said, IMHO the current naming is
> >> not really bad, and I also don't have any suggestions for a perfect name
> >> right now.
> >
> > We don't need a perfect name. We need one that's not actively
> > misleading.
>
> Marc-André, would "qmp/missing-any-arg" and test_missing_any_arg() work
> for you?
Yes, do you want me to update the patch?
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test
2018-10-29 6:49 ` Marc-André Lureau
@ 2018-10-29 8:38 ` Markus Armbruster
0 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2018-10-29 8:38 UTC (permalink / raw)
To: Marc-André Lureau
Cc: Laurent Vivier, Peter Maydell, Thomas Huth, QEMU, Paolo Bonzini
Marc-André Lureau <marcandre.lureau@gmail.com> writes:
> Hi
>
> On Tue, Oct 9, 2018 at 8:41 PM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Markus Armbruster <armbru@redhat.com> writes:
>>
>> > Thomas Huth <thuth@redhat.com> writes:
>> >
>> >> On 2018-08-31 15:24, Marc-André Lureau wrote:
[...]
>> >>> Tbh, I don't care so much about the naming of the test, but (for once)
>> >>> I respectfully don't think Markus suggestion is better.
>> >>>
>> >>> The function checks "qom-set" without 'value' argument:
>> >>> "qom-set-without-value", no brainer..
>> >
>> > Nope, that's not what it tests. It tests the visitor, the marshalling
>> > code generator, and the QMP core handle a certain kind of invalid
>> > arguments correctly. It does not test qom-set. I explained all that
>> > already.
>> >
>> >>> Naming it "invalid-arg" is so generic that I wouldn't be able what it does.
>> >
>> > I can accept "missing-any" or "missing-any-arg". I object to any name
>> > involving qom-set, because the test is not about qom-set at all.
>> >
>> > If it was, then putting it in qmp-test.c would be wrong.
>> >
>> >> Ok, then let's keep it this way. As I said, IMHO the current naming is
>> >> not really bad, and I also don't have any suggestions for a perfect name
>> >> right now.
>> >
>> > We don't need a perfect name. We need one that's not actively
>> > misleading.
>>
>> Marc-André, would "qmp/missing-any-arg" and test_missing_any_arg() work
>> for you?
>
> Yes, do you want me to update the patch?
Yes, please.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PULL 9/9] tests: add a qmp success-response test
2018-08-31 8:38 [Qemu-devel] [PULL 0/9] Removal of deprecated options and improvements for qtests Thomas Huth
` (7 preceding siblings ...)
2018-08-31 8:38 ` [Qemu-devel] [PULL 8/9] tests: add qmp/qom-set-without-value test Thomas Huth
@ 2018-08-31 8:38 ` Thomas Huth
8 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2018-08-31 8:38 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Paolo Bonzini, Laurent Vivier, Eric Blake, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Verify the usage of this schema feature and the API behaviour. This
should be the only case where qmp_dispatch() returns NULL.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/qapi-schema/qapi-schema-test.json | 2 ++
tests/qapi-schema/qapi-schema-test.out | 2 ++
tests/test-qmp-cmds.c | 17 +++++++++++++++++
3 files changed, 21 insertions(+)
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 11aa4c8..fb03163 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -137,6 +137,8 @@
'data': {'ud1a': 'UserDefOne', '*ud1b': 'UserDefOne'},
'returns': 'UserDefTwo' }
+{ 'command': 'cmd-success-response', 'data': {}, 'success-response': false }
+
# Returning a non-dictionary requires a name from the whitelist
{ 'command': 'guest-get-time', 'data': {'a': 'int', '*b': 'int' },
'returns': 'int' }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 0da9245..218ac7d 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -156,6 +156,8 @@ object q_obj_user_def_cmd2-arg
member ud1b: UserDefOne optional=True
command user_def_cmd2 q_obj_user_def_cmd2-arg -> UserDefTwo
gen=True success_response=True boxed=False oob=False preconfig=False
+command cmd-success-response None -> None
+ gen=True success_response=False boxed=False oob=False preconfig=False
object q_obj_guest-get-time-arg
member a: int optional=False
member b: int optional=True
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index ab414fa..4ab2b6e 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -32,6 +32,10 @@ void qmp_test_flags_command(Error **errp)
{
}
+void qmp_cmd_success_response(Error **errp)
+{
+}
+
Empty2 *qmp_user_def_cmd0(Error **errp)
{
return g_new0(Empty2, 1);
@@ -153,6 +157,17 @@ static void test_dispatch_cmd_failure(void)
qobject_unref(req);
}
+static void test_dispatch_cmd_success_response(void)
+{
+ QDict *req = qdict_new();
+ QDict *resp;
+
+ qdict_put_str(req, "execute", "cmd-success-response");
+ resp = qmp_dispatch(&qmp_commands, QOBJECT(req), false);
+ g_assert_null(resp);
+ qobject_unref(req);
+}
+
static QObject *test_qmp_dispatch(QDict *req)
{
QDict *resp;
@@ -289,6 +304,8 @@ int main(int argc, char **argv)
g_test_add_func("/qmp/dispatch_cmd", test_dispatch_cmd);
g_test_add_func("/qmp/dispatch_cmd_failure", test_dispatch_cmd_failure);
g_test_add_func("/qmp/dispatch_cmd_io", test_dispatch_cmd_io);
+ g_test_add_func("/qmp/dispatch_cmd_success_response",
+ test_dispatch_cmd_success_response);
g_test_add_func("/qmp/dealloc_types", test_dealloc_types);
g_test_add_func("/qmp/dealloc_partial", test_dealloc_partial);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread