diff mbox

[16/28] kvm tools: Allow load_flat_binary() to load an initrd alongside

Message ID 4EDD8ED3.8010608@ozlabs.org
State New, archived
Headers show

Commit Message

Matt Evans Dec. 6, 2011, 3:41 a.m. UTC 
This patch passes the initrd fd and commandline to load_flat_binary(), which may
be used to load both the kernel & an initrd (stashing or inserting the
commandline as appropriate) in the same way that load_bzimage() does.  This is
especially useful when load_bzimage() is unused for a particular
architecture. :-)

Signed-off-by: Matt Evans <matt@ozlabs.org>
---
 tools/kvm/include/kvm/kvm.h |    2 +-
 tools/kvm/kvm.c             |   10 ++++++----
 tools/kvm/x86/kvm.c         |   12 +++++++++---
 3 files changed, 16 insertions(+), 8 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Pekka Enberg Dec. 6, 2011, 10:29 a.m. UTC | #1 
On Tue, Dec 6, 2011 at 5:41 AM, Matt Evans <matt@ozlabs.org> wrote:
> This patch passes the initrd fd and commandline to load_flat_binary(), which may
> be used to load both the kernel & an initrd (stashing or inserting the
> commandline as appropriate) in the same way that load_bzimage() does.  This is
> especially useful when load_bzimage() is unused for a particular
> architecture. :-)
>
> Signed-off-by: Matt Evans <matt@ozlabs.org>
> ---
>  tools/kvm/include/kvm/kvm.h |    2 +-
>  tools/kvm/kvm.c             |   10 ++++++----
>  tools/kvm/x86/kvm.c         |   12 +++++++++---
>  3 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
> index fae2ba9..5fe6e75 100644
> --- a/tools/kvm/include/kvm/kvm.h
> +++ b/tools/kvm/include/kvm/kvm.h
> @@ -59,7 +59,7 @@ void kvm__arch_setup_firmware(struct kvm *kvm);
>  bool kvm__arch_cpu_supports_vm(void);
>  void kvm__arch_periodic_poll(struct kvm *kvm);
>
> -int load_flat_binary(struct kvm *kvm, int fd);
> +int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *kernel_cmdline);
>  bool load_bzimage(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *kernel_cmdline, u16 vidmode);
>
>  /*
> diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
> index 457de1a..6f33e1a 100644
> --- a/tools/kvm/kvm.c
> +++ b/tools/kvm/kvm.c
> @@ -354,23 +354,25 @@ bool kvm__load_kernel(struct kvm *kvm, const char *kernel_filename,
>
>        ret = load_bzimage(kvm, fd_kernel, fd_initrd, kernel_cmdline, vidmode);
>
> -       if (initrd_filename)
> -               close(fd_initrd);
> -
>        if (ret)
>                goto found_kernel;
>
>        pr_warning("%s is not a bzImage. Trying to load it as a flat binary...", kernel_filename);
>
> -       ret = load_flat_binary(kvm, fd_kernel);
> +       ret = load_flat_binary(kvm, fd_kernel, fd_initrd, kernel_cmdline);
> +
>        if (ret)
>                goto found_kernel;
>
> +       if (initrd_filename)
> +               close(fd_initrd);
>        close(fd_kernel);
>
>        die("%s is not a valid bzImage or flat binary", kernel_filename);
>
>  found_kernel:
> +       if (initrd_filename)
> +               close(fd_initrd);
>        close(fd_kernel);
>
>        return ret;
> diff --git a/tools/kvm/x86/kvm.c b/tools/kvm/x86/kvm.c
> index 7071dc6..4ac21c0 100644
> --- a/tools/kvm/x86/kvm.c
> +++ b/tools/kvm/x86/kvm.c
> @@ -227,17 +227,23 @@ void kvm__irq_trigger(struct kvm *kvm, int irq)
>  #define BOOT_PROTOCOL_REQUIRED 0x206
>  #define LOAD_HIGH              0x01
>
> -int load_flat_binary(struct kvm *kvm, int fd)
> +int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *kernel_cmdline)
>  {
>        void *p;
>        int nr;
>
> -       if (lseek(fd, 0, SEEK_SET) < 0)
> +       /* Some architectures may support loading an initrd alongside the flat kernel,
> +        * but we do not.
> +        */

Minor nit - we use comment format where the first line is left empty from text:

       /*
        * Some architectures may support loading an initrd alongside
the flat kernel,
        * but we do not.
        */

> +       if (fd_initrd != -1)
> +               pr_warning("Loading initrd with flat binary not supported.");
> +
> +       if (lseek(fd_kernel, 0, SEEK_SET) < 0)
>                die_perror("lseek");
>
>        p = guest_real_to_host(kvm, BOOT_LOADER_SELECTOR, BOOT_LOADER_IP);
>
> -       while ((nr = read(fd, p, 65536)) > 0)
> +       while ((nr = read(fd_kernel, p, 65536)) > 0)
>                p += nr;
>
>        kvm->boot_selector      = BOOT_LOADER_SELECTOR;
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Otherwise looks OK to me. Cyrill?
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cyrill Gorcunov Dec. 6, 2011, 12:04 p.m. UTC | #2 
On Tue, Dec 06, 2011 at 12:29:48PM +0200, Pekka Enberg wrote:
...
> 
> Otherwise looks OK to me. Cyrill?
> 

It might be not seen from patch (or my local kvm repo
is not yet updated well) but I somehow miss who will be
reading initrd in case of loading flat image? If noone,
then what's the point to pass fd there at all?

	Cyrill
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matt Evans Dec. 7, 2011, 12:42 a.m. UTC | #3 
Hi Cyrill,

On 06/12/11 23:04, Cyrill Gorcunov wrote:
> On Tue, Dec 06, 2011 at 12:29:48PM +0200, Pekka Enberg wrote:
> ...
>>
>> Otherwise looks OK to me. Cyrill?
>>
> 
> It might be not seen from patch (or my local kvm repo
> is not yet updated well) but I somehow miss who will be
> reading initrd in case of loading flat image? If noone,
> then what's the point to pass fd there at all?

I pass in the initrd fd in generic code in preparation for the PPC support in
"[PATCH 1/8] kvm tools: Add initial SPAPR PPC64 architecture support" which uses
it.  As you saw, I haven't made x86 load the initrd in the case of a flat kernel
binary being loaded; I didn't think a flat kernel can have a non-embedded initrd
on x86?  (My x86 is rusty, this may not be true ;) bootparams are [b]zImage
only?)


Thanks for reviewing,


Matt
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Cyrill Gorcunov Dec. 7, 2011, 6:33 a.m. UTC | #4 
On Wed, Dec 07, 2011 at 11:42:27AM +1100, Matt Evans wrote:
> Hi Cyrill,
> 
> On 06/12/11 23:04, Cyrill Gorcunov wrote:
> > On Tue, Dec 06, 2011 at 12:29:48PM +0200, Pekka Enberg wrote:
> > ...
> >>
> >> Otherwise looks OK to me. Cyrill?
> >>
> > 
> > It might be not seen from patch (or my local kvm repo
> > is not yet updated well) but I somehow miss who will be
> > reading initrd in case of loading flat image? If noone,
> > then what's the point to pass fd there at all?
> 
> I pass in the initrd fd in generic code in preparation for the PPC support in
> "[PATCH 1/8] kvm tools: Add initial SPAPR PPC64 architecture support" which uses
> it.  As you saw, I haven't made x86 load the initrd in the case of a flat kernel
> binary being loaded; I didn't think a flat kernel can have a non-embedded initrd
> on x86?  (My x86 is rusty, this may not be true ;) bootparams are [b]zImage
> only?)
> 

Ah, I see. Sorry for confusion. Thanks.

	Cyrill
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
index fae2ba9..5fe6e75 100644
--- a/tools/kvm/include/kvm/kvm.h
+++ b/tools/kvm/include/kvm/kvm.h
@@ -59,7 +59,7 @@  void kvm__arch_setup_firmware(struct kvm *kvm);
 bool kvm__arch_cpu_supports_vm(void);
 void kvm__arch_periodic_poll(struct kvm *kvm);
 
-int load_flat_binary(struct kvm *kvm, int fd);
+int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *kernel_cmdline);
 bool load_bzimage(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *kernel_cmdline, u16 vidmode);
 
 /*
diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
index 457de1a..6f33e1a 100644
--- a/tools/kvm/kvm.c
+++ b/tools/kvm/kvm.c
@@ -354,23 +354,25 @@  bool kvm__load_kernel(struct kvm *kvm, const char *kernel_filename,
 
 	ret = load_bzimage(kvm, fd_kernel, fd_initrd, kernel_cmdline, vidmode);
 
-	if (initrd_filename)
-		close(fd_initrd);
-
 	if (ret)
 		goto found_kernel;
 
 	pr_warning("%s is not a bzImage. Trying to load it as a flat binary...", kernel_filename);
 
-	ret = load_flat_binary(kvm, fd_kernel);
+	ret = load_flat_binary(kvm, fd_kernel, fd_initrd, kernel_cmdline);
+
 	if (ret)
 		goto found_kernel;
 
+	if (initrd_filename)
+		close(fd_initrd);
 	close(fd_kernel);
 
 	die("%s is not a valid bzImage or flat binary", kernel_filename);
 
 found_kernel:
+	if (initrd_filename)
+		close(fd_initrd);
 	close(fd_kernel);
 
 	return ret;
diff --git a/tools/kvm/x86/kvm.c b/tools/kvm/x86/kvm.c
index 7071dc6..4ac21c0 100644
--- a/tools/kvm/x86/kvm.c
+++ b/tools/kvm/x86/kvm.c
@@ -227,17 +227,23 @@  void kvm__irq_trigger(struct kvm *kvm, int irq)
 #define BOOT_PROTOCOL_REQUIRED	0x206
 #define LOAD_HIGH		0x01
 
-int load_flat_binary(struct kvm *kvm, int fd)
+int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const char *kernel_cmdline)
 {
 	void *p;
 	int nr;
 
-	if (lseek(fd, 0, SEEK_SET) < 0)
+	/* Some architectures may support loading an initrd alongside the flat kernel,
+	 * but we do not.
+	 */
+	if (fd_initrd != -1)
+		pr_warning("Loading initrd with flat binary not supported.");
+
+	if (lseek(fd_kernel, 0, SEEK_SET) < 0)
 		die_perror("lseek");
 
 	p = guest_real_to_host(kvm, BOOT_LOADER_SELECTOR, BOOT_LOADER_IP);
 
-	while ((nr = read(fd, p, 65536)) > 0)
+	while ((nr = read(fd_kernel, p, 65536)) > 0)
 		p += nr;
 
 	kvm->boot_selector	= BOOT_LOADER_SELECTOR;