[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC v4 PATCH 20/49] multi-process: add qdev_proxy_add to create pro
From: |
Stefan Hajnoczi |
Subject: |
Re: [RFC v4 PATCH 20/49] multi-process: add qdev_proxy_add to create proxy devices |
Date: |
Thu, 21 Nov 2019 12:16:47 +0000 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
On Thu, Oct 24, 2019 at 05:09:01AM -0400, Jagannathan Raman wrote:
> diff --git a/hw/proxy/qemu-proxy.c b/hw/proxy/qemu-proxy.c
> index 3b84055..fc1c731 100644
> --- a/hw/proxy/qemu-proxy.c
> +++ b/hw/proxy/qemu-proxy.c
> @@ -337,7 +337,8 @@ static void init_proxy(PCIDevice *dev, char *command,
> bool need_spawn, Error **e
>
> if (!pdev->managed) {
> if (need_spawn) {
> - if (!remote_spawn(pdev, command, &local_error)) {
> + if (remote_spawn(pdev, command, &local_error)) {
> + fprintf(stderr, "remote spawn failed\n");
> return;
> }
> }
Looks like a fix for a bug in a previous patch. Please squash it.
Also, please propagate local_err and do not use fprintf in a function
that has an errp argument for reporting errors.
> +#if defined(CONFIG_MPQEMU)
Maybe these functions should be in a separate file that the makefile
includes when CONFIG_MPQEMU is defined.
> +
> +static PCIProxyDev *get_proxy_object_rid(const char *rid)
> +{
> + PCIProxyDev *entry;
> + if (!proxy_list_lock.initialized) {
> + QLIST_INIT(&proxy_dev_list.devices);
> + qemu_mutex_init(&proxy_list_lock);
> + }
This locking approach is broken since exactly-once initialization
semantics are required to avoid races during initialization. Is the
lock needed at all?
> +DeviceState *qdev_remote_add(QemuOpts *opts, bool device, Error **errp)
> +{
> + PCIProxyDev *pdev = NULL;
> + DeviceState *dev;
> + const char *rid, *rsocket = NULL, *command = NULL;
> + QDict *qdict_new;
> + const char *id = NULL;
> + const char *driver = NULL;
> + const char *bus = NULL;
> +
> + if (!proxy_list_lock.initialized) {
> + QLIST_INIT(&proxy_dev_list.devices);
> + qemu_mutex_init(&proxy_list_lock);
> + }
> +
> + rid = qemu_opt_get(opts, "rid");
> + if (!rid) {
> + error_setg(errp, "rdevice option needs rid specified.");
> + return NULL;
> + }
> + if (device) {
> + driver = qemu_opt_get(opts, "driver");
> + /* TODO: properly identify the device class. */
> + if (strncmp(driver, "lsi", 3) == 0) {
> + id = qemu_opts_id(opts);
> + if (!id) {
> + error_setg(errp, "qdev_remote_add option needs id
> specified.");
> + return NULL;
> + }
> + }
> + }
> +
> + rsocket = qemu_opt_get(opts, "socket");
> + if (rsocket) {
> + if (strlen(rsocket) > MAX_RID_LENGTH) {
> + error_setg(errp, "Socket number is incorrect.");
> + return NULL;
> + }
> + }
> + /*
> + * TODO: verify command with known commands and on remote end.
> + * How else can we verify the binary we launch without libvirtd support?
> + */
> + command = qemu_opt_get(opts, "command");
> + if (!rsocket && !command) {
> + error_setg(errp, "rdevice option needs socket or command
> specified.");
> + return NULL;
> + }
> +
> + bus = qemu_opt_get(opts, "bus");
> + dev = qdev_proxy_add(rid, id, (char *)bus, (char *)command,
> + rsocket ? atoi(rsocket) : -1,
> + rsocket ? true : false, errp);
> + if (!dev) {
> + error_setg(errp, "qdev_proxy_add error.");
> + return NULL;
> + }
> +
> + qdict_new = qemu_opts_to_qdict(opts, NULL);
> +
> + if (!qdict_new) {
> + error_setg(errp, "Could not parse rdevice options.");
> + return NULL;
> + }
> +
> + pdev = PCI_PROXY_DEV(dev);
> + if (!pdev->set_remote_opts) {
> + /* TODO: destroy proxy? */
> + error_setg(errp, "set_remote_opts failed.");
> + return NULL;
> + } else {
> + if (id && !pdev->dev_id) {
> + pdev->dev_id = g_strdup(id);
> + }
> + pdev->set_remote_opts(PCI_DEVICE(pdev), qdict_new,
> + device ? DEV_OPTS : DRIVE_OPTS);
This function needs to be able to return an error if setting the options
failed. A response message needs to be defined in the protocol to
support this.
Is DRIVE_OPTS still needed? I thought the drives would be configured in
the remote process and no proxy objects were needed on the QEMU side?
signature.asc
Description: PGP signature
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [RFC v4 PATCH 20/49] multi-process: add qdev_proxy_add to create proxy devices,
Stefan Hajnoczi <=