From: Igor Mammedov <imammedo@redhat.com>
To: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: qemu-devel@nongnu.org, rjones@redhat.com, pbonzini@redhat.com,
somlo@cmu.edu, lersek@redhat.com, mst@redhat.com,
ehabkost@redhat.com
Subject: Re: [Qemu-devel] [PATCH 1/6] fw_cfg: move initialisation of FWCfgState into instance_init
Date: Mon, 12 Jun 2017 13:20:34 +0200 [thread overview]
Message-ID: <20170612132034.1d15ab04@nial.brq.redhat.com> (raw)
In-Reply-To: <1497097821-32754-2-git-send-email-mark.cave-ayland@ilande.co.uk>
On Sat, 10 Jun 2017 13:30:16 +0100
Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/nvram/fw_cfg.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 316fca9..144e0c6 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -1017,6 +1017,15 @@ FWCfgState *fw_cfg_find(void)
> return FW_CFG(object_resolve_path(FW_CFG_PATH, NULL));
> }
>
> +static void fw_cfg_init(Object *obj)
> +{
> + FWCfgState *s = FW_CFG(obj);
> +
> + s->entries[0] = g_new0(FWCfgEntry, fw_cfg_max_entry(s));
> + s->entries[1] = g_new0(FWCfgEntry, fw_cfg_max_entry(s));
> + s->entry_order = g_new0(int, fw_cfg_max_entry(s));
it doesn't seem right,
consider,
object_new(fwcfg)
1: fw_cfg_init -> g_new0(FWCfgEntry, fw_cfg_max_entry(s) -> FW_CFG_FILE_SLOTS_DFLT);
2: set property x-file-slots
3: realize -> fw_cfg_file_slots_allocate()
> @@ -1052,10 +1062,6 @@ static void fw_cfg_file_slots_allocate(FWCfgState *s, Error **errp)
> file_slots_max);
> return;
> }
> -
> - s->entries[0] = g_new0(FWCfgEntry, fw_cfg_max_entry(s));
> - s->entries[1] = g_new0(FWCfgEntry, fw_cfg_max_entry(s));
> - s->entry_order = g_new0(int, fw_cfg_max_entry(s));
opps, s->entries doesn't account for new values of x-file-slots
So question is why this patch is needed at all (it needs some reasoning in commit message)?
So for now NACK and I'd suggest to drop this patch.
next prev parent reply other threads:[~2017-06-12 11:20 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-10 12:30 [Qemu-devel] [PATCH 0/6] fw_cfg: qdev-related tidy-ups Mark Cave-Ayland
2017-06-10 12:30 ` [Qemu-devel] [PATCH 1/6] fw_cfg: move initialisation of FWCfgState into instance_init Mark Cave-Ayland
2017-06-10 18:05 ` Philippe Mathieu-Daudé
2017-06-12 11:20 ` Igor Mammedov [this message]
2017-06-12 11:45 ` Mark Cave-Ayland
2017-06-12 18:27 ` Laszlo Ersek
2017-06-12 18:46 ` Mark Cave-Ayland
2017-06-10 12:30 ` [Qemu-devel] [PATCH 2/6] fw_cfg: move qdev_init_nofail() out from fw_cfg_init1() into callers Mark Cave-Ayland
2017-06-12 11:43 ` Igor Mammedov
2017-06-12 11:54 ` Mark Cave-Ayland
2017-06-12 19:15 ` Laszlo Ersek
2017-06-12 19:50 ` Mark Cave-Ayland
2017-06-12 21:11 ` Mark Cave-Ayland
2017-06-10 12:30 ` [Qemu-devel] [PATCH 3/6] fw_cfg: move setting of FW_CFG_ID into fw_cfg_init1() Mark Cave-Ayland
2017-06-10 18:07 ` Philippe Mathieu-Daudé
2017-06-12 11:37 ` Igor Mammedov
2017-06-12 11:49 ` Mark Cave-Ayland
2017-06-12 13:03 ` Gerd Hoffmann
2017-06-10 12:30 ` [Qemu-devel] [PATCH 4/6] fw_cfg: move fw_cfg_init1() into the fw_cfg_*_realize() functions Mark Cave-Ayland
2017-06-12 12:03 ` Igor Mammedov
2017-06-10 12:30 ` [Qemu-devel] [PATCH 5/6] fw_cfg: use sysbus_init_mmio() in fw_cfg_io_realize() Mark Cave-Ayland
2017-06-12 12:16 ` Igor Mammedov
2017-06-12 12:59 ` Mark Cave-Ayland
2017-06-10 12:30 ` [Qemu-devel] [PATCH 6/6] fw_cfg: move QOM type defines into fw_cfg.h Mark Cave-Ayland
2017-06-10 17:43 ` Philippe Mathieu-Daudé
2017-06-10 18:15 ` [Qemu-devel] [PATCH 0/6] fw_cfg: qdev-related tidy-ups Philippe Mathieu-Daudé
2017-06-11 15:00 ` Mark Cave-Ayland
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170612132034.1d15ab04@nial.brq.redhat.com \
--to=imammedo@redhat.com \
--cc=ehabkost@redhat.com \
--cc=lersek@redhat.com \
--cc=mark.cave-ayland@ilande.co.uk \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=rjones@redhat.com \
--cc=somlo@cmu.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).