All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: "Gabriel L. Somlo" <gsomlo@gmail.com>
Cc: matt.fleming@intel.com, mdroth@linux.vnet.ibm.com,
	rjones@redhat.com, jordan.l.justen@intel.com,
	"Gabriel L. Somlo" <somlo@cmu.edu>,
	gleb@cloudius-systems.com, qemu-devel@nongnu.org,
	Gerd Hoffmann <kraxel@redhat.com>,
	pbonzini@redhat.com, Markus Armbruster <armbru@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 5/6] fw_cfg: insert fw_cfg file blobs via qemu cmdline
Date: Thu, 19 Mar 2015 11:43:36 +0100	[thread overview]
Message-ID: <550AA858.3060005@redhat.com> (raw)
In-Reply-To: <20150318200619.GL1832@HEDWIG.INI.CMU.EDU>

On 03/18/15 21:06, Gabriel L. Somlo wrote:
> On Tue, Mar 17, 2015 at 12:49:50PM +0100, Gerd Hoffmann wrote:

[snip]

>> Which reminds me:  Going for QemuOpts would be very useful (gives
>> -readconfig support), and it would solve the init order issue too.
>> Instead of having your custom storage you just let QemuOpts parse and
>> store the command line switch, then process them after fw_cfg device
>> initialization.

[snip]

> So basically, you're saying "don't process 'opts' here with qemu_opt_get(),
> just stash them as is, then process them later" ?  (e.g. during
> fw_cfg_options_insert() below) ?
> 
> Is there an existing example where that's currently done that I use
> for inspiration?
> 
> Laszlo's suggestion was almost the opposite of this, i.e. allocate
> memory for all the blobs during option processing, them just call
> fw_cfg_add_file() during fw_cfg_options_insert().

I was wrong, simply. :) I forgot about -readconfig completely.

(I do recall that I had been annoyed earlier by some option not working
as expected with -readconfig; perhaps -smp? Not sure.)

Meta:

You should always take my qemu reviews with a grain of salt. They're
mostly good for pulling in reviewers who know what they're doing. ;)

I reviewed your v1 quickly for two reasons:
- I didn't want you to regress fw_cfg accidentally, especially code I
  wrote, or code UEFI depends on.
- I'm a nice person (lol) and wanted to get the review gears rolling
  for you (see above).

Cheers,
Laszlo

  reply	other threads:[~2015-03-19 10:44 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-16 14:14 [Qemu-devel] [PATCH 0/6] fw-cfg: documentation, cleanup, and proposed feature Gabriel L. Somlo
2015-03-16 14:15 ` [Qemu-devel] [PATCH 1/6] fw_cfg: add documentation file (docs/specs/fw_cfg.txt) Gabriel L. Somlo
2015-03-16 16:30   ` Laszlo Ersek
2015-03-16 14:15 ` [Qemu-devel] [PATCH 2/6] fw_cfg: remove support for guest-side data writes Gabriel L. Somlo
2015-03-16 17:02   ` Laszlo Ersek
2015-03-16 18:41     ` Gabriel L. Somlo
2015-03-17  7:46       ` Gerd Hoffmann
2015-03-16 14:15 ` [Qemu-devel] [PATCH 3/6] fw_cfg: assertion to detect memory leak when adding new data blob Gabriel L. Somlo
2015-03-16 19:12   ` Laszlo Ersek
2015-03-16 14:15 ` [Qemu-devel] [PATCH 4/6] fw_cfg: exit with error when dupe fw_cfg file name inserted Gabriel L. Somlo
2015-03-16 19:26   ` Laszlo Ersek
2015-03-16 14:15 ` [Qemu-devel] [PATCH 5/6] fw_cfg: insert fw_cfg file blobs via qemu cmdline Gabriel L. Somlo
2015-03-17 10:07   ` Gerd Hoffmann
2015-03-17 10:55   ` Matt Fleming
2015-03-17 14:09     ` Gabriel L. Somlo
2015-03-17 11:28   ` Laszlo Ersek
2015-03-17 11:49     ` Gerd Hoffmann
2015-03-18 20:06       ` Gabriel L. Somlo
2015-03-19 10:43         ` Laszlo Ersek [this message]
2015-03-18 20:27     ` Gabriel L. Somlo
2015-03-19  7:34       ` Gerd Hoffmann
2015-03-19  8:41       ` [Qemu-devel] How to emit errors with nice location information (was: [PATCH 5/6] fw_cfg: insert fw_cfg file blobs via qemu cmdline) Markus Armbruster
2015-03-16 14:15 ` [Qemu-devel] [PATCH 6/6] qga: RFC: guest-side retrieval of fw_cfg file Gabriel L. Somlo
2015-03-17 12:38   ` Laszlo Ersek
2015-03-17 14:28     ` Gabriel L. Somlo
2015-03-19 18:27     ` Kevin O'Connor
2015-03-19 18:44       ` Laszlo Ersek
2015-03-16 14:26 ` [Qemu-devel] [PATCH 0/6] fw-cfg: documentation, cleanup, and proposed feature Patchew Tool

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=550AA858.3060005@redhat.com \
    --to=lersek@redhat.com \
    --cc=armbru@redhat.com \
    --cc=gleb@cloudius-systems.com \
    --cc=gsomlo@gmail.com \
    --cc=jordan.l.justen@intel.com \
    --cc=kraxel@redhat.com \
    --cc=matt.fleming@intel.com \
    --cc=mdroth@linux.vnet.ibm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.