From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:32863)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mjt@tls.msk.ru>) id 1TPpvy-0001Y1-SU
	for qemu-devel@nongnu.org; Sun, 21 Oct 2012 03:26:55 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mjt@tls.msk.ru>) id 1TPpvx-0005Ua-QP
	for qemu-devel@nongnu.org; Sun, 21 Oct 2012 03:26:54 -0400
Received: from isrv.corpit.ru ([86.62.121.231]:60478)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mjt@tls.msk.ru>) id 1TPpvx-0005UF-F7
	for qemu-devel@nongnu.org; Sun, 21 Oct 2012 03:26:53 -0400
Message-ID: <5083A3B9.90708@msgid.tls.msk.ru>
Date: Sun, 21 Oct 2012 11:26:49 +0400
From: Michael Tokarev <mjt@tls.msk.ru>
MIME-Version: 1.0
References: <cover.1350677360.git.jbaron@redhat.com>
	<38e5ef70b494a5e21fdb89b2105e9a87fd410f7c.1350677362.git.jbaron@redhat.com>
In-Reply-To: <38e5ef70b494a5e21fdb89b2105e9a87fd410f7c.1350677362.git.jbaron@redhat.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v3 22/26] Add a fallback bios file search,
 if -L fails.
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Jason Baron <jbaron@redhat.com>
Cc: agraf@suse.de, aliguori@us.ibm.com, alex.williamson@redhat.com, mst@redhat.com, jan.kiszka@siemens.com, qemu-devel@nongnu.org, armbru@redhat.com, blauwirbel@gmail.com, yamahata@valinux.co.jp, juzhang@redhat.com, kevin@koconnor.net, avi@redhat.com, mkletzan@redhat.com, pbonzini@redhat.com, lcapitulino@redhat.com, afaerber@suse.de, kraxel@redhat.com

On 20.10.2012 00:43, Jason Baron wrote:
> From: Jason Baron <jbaron@redhat.com>
> 
> If -L <dir> is specified, and qemu does not find the bios file in <dir>, then
> the search fails. Add infrastructure such that the search will continue in
> the default paths, if not found in the -L path.
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Jason Baron <jbaron@redhat.com>
> ---
>  vl.c |   36 +++++++++++++++++++++++++-----------
>  1 files changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 6b1e546..2a2d217 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -177,6 +177,7 @@ int main(int argc, char **argv)
>  #define MAX_VIRTIO_CONSOLES 1
>  
>  static const char *data_dir;
> +static const char *data_dir_fallback;
>  const char *bios_name = NULL;
>  enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
>  DisplayType display_type = DT_DEFAULT;
> @@ -1892,16 +1893,16 @@ static int balloon_parse(const char *arg)
>      return -1;
>  }
>  
> -char *qemu_find_file(int type, const char *name)
> +static char *qemu_find_file_in_dir(int type, const char *name, const char *dir)
>  {
>      int len;
>      const char *subdir;
>      char *buf;
>  
> -    /* Try the name as a straight path first */
> -    if (access(name, R_OK) == 0) {
> -        return g_strdup(name);
> +    if (!dir) {
> +        return NULL;
>      }
> +
>      switch (type) {
>      case QEMU_FILE_TYPE_BIOS:
>          subdir = "";
> @@ -1912,9 +1913,9 @@ char *qemu_find_file(int type, const char *name)
>      default:
>          abort();
>      }
> -    len = strlen(data_dir) + strlen(name) + strlen(subdir) + 2;
> +    len = strlen(dir) + strlen(name) + strlen(subdir) + 2;
>      buf = g_malloc0(len);
> -    snprintf(buf, len, "%s/%s%s", data_dir, subdir, name);
> +    snprintf(buf, len, "%s/%s%s", dir, subdir, name);
>      if (access(buf, R_OK)) {
>          g_free(buf);
>          return NULL;
> @@ -1922,6 +1923,21 @@ char *qemu_find_file(int type, const char *name)
>      return buf;
>  }
>  
> +char *qemu_find_file(int type, const char *name)
> +{
> +    char *filename;
> +
> +    /* Try the name as a straight path first */
> +    if (access(name, R_OK) == 0) {
> +        return g_strdup(name);
> +    }

FWIW, this can be a security issue, when a more privileged
user tries to run qemu from trusted path (/usr/bin) in a
directory owned by non-privileged user, to utilize -runas
or somesuch.  I understand it's been this way since the
beginning.

Maybe we can do a bit better here, like (windows systems
aside) this:

    if (strchr(name, '/') && access(name, R_OK) == 0) {...}

Note that -L can be set to "." in order to check in current
directory too.

And going forward, maybe we can treat -L argument as a PATH
(separated by whatever delimiter is okay) instead of just
a single directory?  This way, for example, we can put
various kinds of data (pxe roms, vgabios roms etc) into
different places and qemu will be able to find them all.

I understand this whole thing is not really related to q35
changes, so can be pushed/applied separately.

Thanks,

/mjt