diff mbox

[v3] ui/cocoa.m: Add Mount image file menu item

Message ID 6AF094C9-59D3-40E4-AFAE-ECAE1D2A8414@gmail.com
State New
Headers show

Commit Message

Programmingkid Sept. 18, 2015, 4:17 a.m. UTC
Add "Mount Image File..." and a "Eject Image File" menu items to
cocoa interface. This patch makes sharing files between the
host and the guest user-friendly.

The "Mount Image File..." menu item displays a dialog box having the
user pick an image file to use in QEMU. The image file is setup as
a USB flash drive. The user can do the equivalent of removing the
flash drive by selecting the file in the "Eject Image File" submenu.

Signed-off-by: John Arbuckle <programmingkidx@gmail.com>

---
Removed usbAvailable() function.
Detects if an USB bus is available.
Prevents the user from trying to use an image file that is already in use.
Moved variable declarations to the start of their blocks.
Changed variable names to match QEMU naming convention.
Fixed several memory leaks. 

 ui/cocoa.m |  252 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 251 insertions(+), 1 deletions(-)

Comments

Peter Maydell Sept. 23, 2015, 8:35 p.m. UTC | #1
On 17 September 2015 at 21:17, Programmingkid <programmingkidx@gmail.com> wrote:
> Add "Mount Image File..." and a "Eject Image File" menu items to
> cocoa interface. This patch makes sharing files between the
> host and the guest user-friendly.
>
> The "Mount Image File..." menu item displays a dialog box having the
> user pick an image file to use in QEMU. The image file is setup as
> a USB flash drive. The user can do the equivalent of removing the
> flash drive by selecting the file in the "Eject Image File" submenu.
>
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>

I've thought a bit about this, and I really don't think this sort
of feature should be part of QEMU itself. Our general design for
how QEMU does this sort of thing is that an external program
(virt-manager, for instance) is responsible for providing most
of the UI conveniences the user wants, and QEMU's "ui" code is
a fairly simple minimum-functionality affair. I agree with Markus
that this separation of concerns has generally worked OK for us.

I don't think OSX should be an exception to this design model:
 (a) being an odd special case is never a good idea
 (b) as a practical matter, I'm the only person who really reviews
OSX patches, and I don't have either the time nor the UI or OSX
expertise to deal with maintaining what will effectively be a
vm-manager grafted onto the side of QEMU

So I think your efforts would be better spent in either porting
one of the Linux frontends like libvirt/virt-manager, or in
writing a custom OSX specific frontend.

thanks
-- PMM
Programmingkid Sept. 23, 2015, 10 p.m. UTC | #2
On Sep 23, 2015, at 4:35 PM, Peter Maydell wrote:

> On 17 September 2015 at 21:17, Programmingkid <programmingkidx@gmail.com> wrote:
>> Add "Mount Image File..." and a "Eject Image File" menu items to
>> cocoa interface. This patch makes sharing files between the
>> host and the guest user-friendly.
>> 
>> The "Mount Image File..." menu item displays a dialog box having the
>> user pick an image file to use in QEMU. The image file is setup as
>> a USB flash drive. The user can do the equivalent of removing the
>> flash drive by selecting the file in the "Eject Image File" submenu.
>> 
>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
> 
> I've thought a bit about this, and I really don't think this sort
> of feature should be part of QEMU itself. Our general design for
> how QEMU does this sort of thing is that an external program
> (virt-manager, for instance) is responsible for providing most
> of the UI conveniences the user wants, and QEMU's "ui" code is
> a fairly simple minimum-functionality affair. I agree with Markus
> that this separation of concerns has generally worked OK for us.
> 
> I don't think OSX should be an exception to this design model:
> (a) being an odd special case is never a good idea
> (b) as a practical matter, I'm the only person who really reviews
> OSX patches, and I don't have either the time nor the UI or OSX
> expertise to deal with maintaining what will effectively be a
> vm-manager grafted onto the side of QEMU
> 
> So I think your efforts would be better spent in either porting
> one of the Linux frontends like libvirt/virt-manager, or in
> writing a custom OSX specific frontend.

I understand that time is precious. It is one of those things
that we only have a finite amount of. Every user can agree
to that. This patch was pretty hairy looking with the QDict
and other unfamiliar code. With that said I'm not ready to
give up on this patch. It is a huge time saver for the user.
Without it, the user would need to spend a lot of time
investigating documentation. What's worse is the user
would have to type out full paths to files they need. This
would definitely be error prone and frustrating.

This patch can definitely be more simplified. QMP
commands could be used in place of C functions. 
This would reduce the patch size greatly.
Markus Armbruster Sept. 24, 2015, 6:57 a.m. UTC | #3
Programmingkid <programmingkidx@gmail.com> writes:

> On Sep 23, 2015, at 4:35 PM, Peter Maydell wrote:
>
>> On 17 September 2015 at 21:17, Programmingkid
>> <programmingkidx@gmail.com> wrote:
>>> Add "Mount Image File..." and a "Eject Image File" menu items to
>>> cocoa interface. This patch makes sharing files between the
>>> host and the guest user-friendly.
>>> 
>>> The "Mount Image File..." menu item displays a dialog box having the
>>> user pick an image file to use in QEMU. The image file is setup as
>>> a USB flash drive. The user can do the equivalent of removing the
>>> flash drive by selecting the file in the "Eject Image File" submenu.
>>> 
>>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>> 
>> I've thought a bit about this, and I really don't think this sort
>> of feature should be part of QEMU itself. Our general design for
>> how QEMU does this sort of thing is that an external program
>> (virt-manager, for instance) is responsible for providing most
>> of the UI conveniences the user wants, and QEMU's "ui" code is
>> a fairly simple minimum-functionality affair. I agree with Markus
>> that this separation of concerns has generally worked OK for us.
>> 
>> I don't think OSX should be an exception to this design model:
>> (a) being an odd special case is never a good idea
>> (b) as a practical matter, I'm the only person who really reviews
>> OSX patches, and I don't have either the time nor the UI or OSX
>> expertise to deal with maintaining what will effectively be a
>> vm-manager grafted onto the side of QEMU
>> 
>> So I think your efforts would be better spent in either porting
>> one of the Linux frontends like libvirt/virt-manager, or in
>> writing a custom OSX specific frontend.
>
> I understand that time is precious. It is one of those things
> that we only have a finite amount of. Every user can agree
> to that. This patch was pretty hairy looking with the QDict
> and other unfamiliar code. With that said I'm not ready to
> give up on this patch. It is a huge time saver for the user.
> Without it, the user would need to spend a lot of time
> investigating documentation. What's worse is the user
> would have to type out full paths to files they need. This
> would definitely be error prone and frustrating.

Nobody is challenging the idea that many users appreciate a GUI.

What we've been trying to tell you is where in this software layer cake
the GUI should be.  In Peter's words, "our general design for how QEMU
does this sort of thing is that an external program (virt-manager, for
instance) is responsible for providing most of the UI conveniences".

> This patch can definitely be more simplified. QMP
> commands could be used in place of C functions. 
> This would reduce the patch size greatly. 

You're quite welcome to use QMP the way it wants to be used: as an
external interface.

Abusing it as internal interface won't fly.
Programmingkid Sept. 24, 2015, 11:55 a.m. UTC | #4
On Sep 24, 2015, at 2:57 AM, Markus Armbruster wrote:

> Programmingkid <programmingkidx@gmail.com> writes:
> 
>> On Sep 23, 2015, at 4:35 PM, Peter Maydell wrote:
>> 
>>> On 17 September 2015 at 21:17, Programmingkid
>>> <programmingkidx@gmail.com> wrote:
>>>> Add "Mount Image File..." and a "Eject Image File" menu items to
>>>> cocoa interface. This patch makes sharing files between the
>>>> host and the guest user-friendly.
>>>> 
>>>> The "Mount Image File..." menu item displays a dialog box having the
>>>> user pick an image file to use in QEMU. The image file is setup as
>>>> a USB flash drive. The user can do the equivalent of removing the
>>>> flash drive by selecting the file in the "Eject Image File" submenu.
>>>> 
>>>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>>> 
>>> I've thought a bit about this, and I really don't think this sort
>>> of feature should be part of QEMU itself. Our general design for
>>> how QEMU does this sort of thing is that an external program
>>> (virt-manager, for instance) is responsible for providing most
>>> of the UI conveniences the user wants, and QEMU's "ui" code is
>>> a fairly simple minimum-functionality affair. I agree with Markus
>>> that this separation of concerns has generally worked OK for us.
>>> 
>>> I don't think OSX should be an exception to this design model:
>>> (a) being an odd special case is never a good idea
>>> (b) as a practical matter, I'm the only person who really reviews
>>> OSX patches, and I don't have either the time nor the UI or OSX
>>> expertise to deal with maintaining what will effectively be a
>>> vm-manager grafted onto the side of QEMU
>>> 
>>> So I think your efforts would be better spent in either porting
>>> one of the Linux frontends like libvirt/virt-manager, or in
>>> writing a custom OSX specific frontend.
>> 
>> I understand that time is precious. It is one of those things
>> that we only have a finite amount of. Every user can agree
>> to that. This patch was pretty hairy looking with the QDict
>> and other unfamiliar code. With that said I'm not ready to
>> give up on this patch. It is a huge time saver for the user.
>> Without it, the user would need to spend a lot of time
>> investigating documentation. What's worse is the user
>> would have to type out full paths to files they need. This
>> would definitely be error prone and frustrating.
> 
> Nobody is challenging the idea that many users appreciate a GUI.
> 
> What we've been trying to tell you is where in this software layer cake
> the GUI should be.  In Peter's words, "our general design for how QEMU
> does this sort of thing is that an external program (virt-manager, for
> instance) is responsible for providing most of the UI conveniences".

That is easy for you to say. Linux already has virt-manager. Mac OS X doesn't. 
Expecting someone to just go and port another program to Mac OS X is 
unreasonable. The amount of time and energy it would take to do so
would make it hard. 

> 
>> This patch can definitely be more simplified. QMP
>> commands could be used in place of C functions. 
>> This would reduce the patch size greatly. 
> 
> You're quite welcome to use QMP the way it wants to be used: as an
> external interface.
> 
> Abusing it as internal interface won't fly.

The QMP interface is primarily there to help a gui interact with QEMU. That
is what I intend to use it for.
Markus Armbruster Sept. 25, 2015, 3:42 p.m. UTC | #5
Programmingkid <programmingkidx@gmail.com> writes:

> On Sep 24, 2015, at 2:57 AM, Markus Armbruster wrote:
>
>> Programmingkid <programmingkidx@gmail.com> writes:
>> 
>>> On Sep 23, 2015, at 4:35 PM, Peter Maydell wrote:
>>> 
>>>> On 17 September 2015 at 21:17, Programmingkid
>>>> <programmingkidx@gmail.com> wrote:
>>>>> Add "Mount Image File..." and a "Eject Image File" menu items to
>>>>> cocoa interface. This patch makes sharing files between the
>>>>> host and the guest user-friendly.
>>>>> 
>>>>> The "Mount Image File..." menu item displays a dialog box having the
>>>>> user pick an image file to use in QEMU. The image file is setup as
>>>>> a USB flash drive. The user can do the equivalent of removing the
>>>>> flash drive by selecting the file in the "Eject Image File" submenu.
>>>>> 
>>>>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>>>> 
>>>> I've thought a bit about this, and I really don't think this sort
>>>> of feature should be part of QEMU itself. Our general design for
>>>> how QEMU does this sort of thing is that an external program
>>>> (virt-manager, for instance) is responsible for providing most
>>>> of the UI conveniences the user wants, and QEMU's "ui" code is
>>>> a fairly simple minimum-functionality affair. I agree with Markus
>>>> that this separation of concerns has generally worked OK for us.
>>>> 
>>>> I don't think OSX should be an exception to this design model:
>>>> (a) being an odd special case is never a good idea
>>>> (b) as a practical matter, I'm the only person who really reviews
>>>> OSX patches, and I don't have either the time nor the UI or OSX
>>>> expertise to deal with maintaining what will effectively be a
>>>> vm-manager grafted onto the side of QEMU
>>>> 
>>>> So I think your efforts would be better spent in either porting
>>>> one of the Linux frontends like libvirt/virt-manager, or in
>>>> writing a custom OSX specific frontend.
>>> 
>>> I understand that time is precious. It is one of those things
>>> that we only have a finite amount of. Every user can agree
>>> to that. This patch was pretty hairy looking with the QDict
>>> and other unfamiliar code. With that said I'm not ready to
>>> give up on this patch. It is a huge time saver for the user.
>>> Without it, the user would need to spend a lot of time
>>> investigating documentation. What's worse is the user
>>> would have to type out full paths to files they need. This
>>> would definitely be error prone and frustrating.
>> 
>> Nobody is challenging the idea that many users appreciate a GUI.
>> 
>> What we've been trying to tell you is where in this software layer cake
>> the GUI should be.  In Peter's words, "our general design for how QEMU
>> does this sort of thing is that an external program (virt-manager, for
>> instance) is responsible for providing most of the UI conveniences".
>
> That is easy for you to say. Linux already has virt-manager. Mac OS X doesn't. 
> Expecting someone to just go and port another program to Mac OS X is 
> unreasonable. The amount of time and energy it would take to do so
> would make it hard. 

On the purely technical level, it may or may not be harder than mashing
everything into QEMU.

On the getting-patches-merged level, mashing everything into QEMU is a
non-starter, as Peter and I have told you multiple times.

That tips the balance somewhat.

>>> This patch can definitely be more simplified. QMP
>>> commands could be used in place of C functions. 
>>> This would reduce the patch size greatly. 
>> 
>> You're quite welcome to use QMP the way it wants to be used: as an
>> external interface.
>> 
>> Abusing it as internal interface won't fly.
>
> The QMP interface is primarily there to help a gui interact with QEMU. That
> is what I intend to use it for.

Nope, the QMP interface's purpose is to let other programs interact with
QEMU.

You're free to use it for other purposes to your heart's content.  Just
don't count on patches to be merged when they do things maintainers have
told you not to do :)
Programmingkid Sept. 25, 2015, 4:15 p.m. UTC | #6
On Sep 23, 2015, at 4:35 PM, Peter Maydell wrote:

> On 17 September 2015 at 21:17, Programmingkid <programmingkidx@gmail.com> wrote:
>> Add "Mount Image File..." and a "Eject Image File" menu items to
>> cocoa interface. This patch makes sharing files between the
>> host and the guest user-friendly.
>> 
>> The "Mount Image File..." menu item displays a dialog box having the
>> user pick an image file to use in QEMU. The image file is setup as
>> a USB flash drive. The user can do the equivalent of removing the
>> flash drive by selecting the file in the "Eject Image File" submenu.
>> 
>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
> 
> I've thought a bit about this, and I really don't think this sort
> of feature should be part of QEMU itself. Our general design for
> how QEMU does this sort of thing is that an external program
> (virt-manager, for instance) is responsible for providing most
> of the UI conveniences the user wants, and QEMU's "ui" code is
> a fairly simple minimum-functionality affair. I agree with Markus
> that this separation of concerns has generally worked OK for us.
> 
> I don't think OSX should be an exception to this design model:
> (a) being an odd special case is never a good idea
> (b) as a practical matter, I'm the only person who really reviews
> OSX patches, and I don't have either the time nor the UI or OSX
> expertise to deal with maintaining what will effectively be a
> vm-manager grafted onto the side of QEMU
> 
> So I think your efforts would be better spent in either porting
> one of the Linux frontends like libvirt/virt-manager, or in
> writing a custom OSX specific frontend.

Given the overwhelming odds of this patch being committed, I have
decided to give up on it. 

I guess it is time for plan B - make the SD card reader actually work. 
Would you know what operating systems or targets can actually use
the SD card reader device in QEMU?
Programmingkid Sept. 25, 2015, 5:04 p.m. UTC | #7
On Sep 25, 2015, at 11:42 AM, Markus Armbruster wrote:

> Programmingkid <programmingkidx@gmail.com> writes:
> 
>> On Sep 24, 2015, at 2:57 AM, Markus Armbruster wrote:
>> 
>>> Programmingkid <programmingkidx@gmail.com> writes:
>>> 
>>>> On Sep 23, 2015, at 4:35 PM, Peter Maydell wrote:
>>>> 
>>>>> On 17 September 2015 at 21:17, Programmingkid
>>>>> <programmingkidx@gmail.com> wrote:
>>>>>> Add "Mount Image File..." and a "Eject Image File" menu items to
>>>>>> cocoa interface. This patch makes sharing files between the
>>>>>> host and the guest user-friendly.
>>>>>> 
>>>>>> The "Mount Image File..." menu item displays a dialog box having the
>>>>>> user pick an image file to use in QEMU. The image file is setup as
>>>>>> a USB flash drive. The user can do the equivalent of removing the
>>>>>> flash drive by selecting the file in the "Eject Image File" submenu.
>>>>>> 
>>>>>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>>>>> 
>>>>> I've thought a bit about this, and I really don't think this sort
>>>>> of feature should be part of QEMU itself. Our general design for
>>>>> how QEMU does this sort of thing is that an external program
>>>>> (virt-manager, for instance) is responsible for providing most
>>>>> of the UI conveniences the user wants, and QEMU's "ui" code is
>>>>> a fairly simple minimum-functionality affair. I agree with Markus
>>>>> that this separation of concerns has generally worked OK for us.
>>>>> 
>>>>> I don't think OSX should be an exception to this design model:
>>>>> (a) being an odd special case is never a good idea
>>>>> (b) as a practical matter, I'm the only person who really reviews
>>>>> OSX patches, and I don't have either the time nor the UI or OSX
>>>>> expertise to deal with maintaining what will effectively be a
>>>>> vm-manager grafted onto the side of QEMU
>>>>> 
>>>>> So I think your efforts would be better spent in either porting
>>>>> one of the Linux frontends like libvirt/virt-manager, or in
>>>>> writing a custom OSX specific frontend.
>>>> 
>>>> I understand that time is precious. It is one of those things
>>>> that we only have a finite amount of. Every user can agree
>>>> to that. This patch was pretty hairy looking with the QDict
>>>> and other unfamiliar code. With that said I'm not ready to
>>>> give up on this patch. It is a huge time saver for the user.
>>>> Without it, the user would need to spend a lot of time
>>>> investigating documentation. What's worse is the user
>>>> would have to type out full paths to files they need. This
>>>> would definitely be error prone and frustrating.
>>> 
>>> Nobody is challenging the idea that many users appreciate a GUI.
>>> 
>>> What we've been trying to tell you is where in this software layer cake
>>> the GUI should be.  In Peter's words, "our general design for how QEMU
>>> does this sort of thing is that an external program (virt-manager, for
>>> instance) is responsible for providing most of the UI conveniences".
>> 
>> That is easy for you to say. Linux already has virt-manager. Mac OS X doesn't. 
>> Expecting someone to just go and port another program to Mac OS X is 
>> unreasonable. The amount of time and energy it would take to do so
>> would make it hard. 
> 
> On the purely technical level, it may or may not be harder than mashing
> everything into QEMU.
> 
> On the getting-patches-merged level, mashing everything into QEMU is a
> non-starter, as Peter and I have told you multiple times.
> 
> That tips the balance somewhat.
> 
>>>> This patch can definitely be more simplified. QMP
>>>> commands could be used in place of C functions. 
>>>> This would reduce the patch size greatly. 
>>> 
>>> You're quite welcome to use QMP the way it wants to be used: as an
>>> external interface.
>>> 
>>> Abusing it as internal interface won't fly.
>> 
>> The QMP interface is primarily there to help a gui interact with QEMU. That
>> is what I intend to use it for.
> 
> Nope, the QMP interface's purpose is to let other programs interact with
> QEMU.
> 
> You're free to use it for other purposes to your heart's content.  Just
> don't count on patches to be merged when they do things maintainers have
> told you not to do :)

I did do as you said and used C functions in place of the original hmp commands. 
I guess there never was any hope for this patch. :(
Markus Armbruster Sept. 28, 2015, 8:09 p.m. UTC | #8
Programmingkid <programmingkidx@gmail.com> writes:

> On Sep 25, 2015, at 11:42 AM, Markus Armbruster wrote:
>
>> Programmingkid <programmingkidx@gmail.com> writes:
>> 
>>> On Sep 24, 2015, at 2:57 AM, Markus Armbruster wrote:
>>> 
>>>> Programmingkid <programmingkidx@gmail.com> writes:
>>>> 
>>>>> On Sep 23, 2015, at 4:35 PM, Peter Maydell wrote:
>>>>> 
>>>>>> On 17 September 2015 at 21:17, Programmingkid
>>>>>> <programmingkidx@gmail.com> wrote:
>>>>>>> Add "Mount Image File..." and a "Eject Image File" menu items to
>>>>>>> cocoa interface. This patch makes sharing files between the
>>>>>>> host and the guest user-friendly.
>>>>>>> 
>>>>>>> The "Mount Image File..." menu item displays a dialog box having the
>>>>>>> user pick an image file to use in QEMU. The image file is setup as
>>>>>>> a USB flash drive. The user can do the equivalent of removing the
>>>>>>> flash drive by selecting the file in the "Eject Image File" submenu.
>>>>>>> 
>>>>>>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>>>>>> 
>>>>>> I've thought a bit about this, and I really don't think this sort
>>>>>> of feature should be part of QEMU itself. Our general design for
>>>>>> how QEMU does this sort of thing is that an external program
>>>>>> (virt-manager, for instance) is responsible for providing most
>>>>>> of the UI conveniences the user wants, and QEMU's "ui" code is
>>>>>> a fairly simple minimum-functionality affair. I agree with Markus
>>>>>> that this separation of concerns has generally worked OK for us.
>>>>>> 
>>>>>> I don't think OSX should be an exception to this design model:
>>>>>> (a) being an odd special case is never a good idea
>>>>>> (b) as a practical matter, I'm the only person who really reviews
>>>>>> OSX patches, and I don't have either the time nor the UI or OSX
>>>>>> expertise to deal with maintaining what will effectively be a
>>>>>> vm-manager grafted onto the side of QEMU
>>>>>> 
>>>>>> So I think your efforts would be better spent in either porting
>>>>>> one of the Linux frontends like libvirt/virt-manager, or in
>>>>>> writing a custom OSX specific frontend.
>>>>> 
>>>>> I understand that time is precious. It is one of those things
>>>>> that we only have a finite amount of. Every user can agree
>>>>> to that. This patch was pretty hairy looking with the QDict
>>>>> and other unfamiliar code. With that said I'm not ready to
>>>>> give up on this patch. It is a huge time saver for the user.
>>>>> Without it, the user would need to spend a lot of time
>>>>> investigating documentation. What's worse is the user
>>>>> would have to type out full paths to files they need. This
>>>>> would definitely be error prone and frustrating.
>>>> 
>>>> Nobody is challenging the idea that many users appreciate a GUI.
>>>> 
>>>> What we've been trying to tell you is where in this software layer cake
>>>> the GUI should be.  In Peter's words, "our general design for how QEMU
>>>> does this sort of thing is that an external program (virt-manager, for
>>>> instance) is responsible for providing most of the UI conveniences".
>>> 
>>> That is easy for you to say. Linux already has virt-manager. Mac OS
>>> X doesn't.
>>> Expecting someone to just go and port another program to Mac OS X is 
>>> unreasonable. The amount of time and energy it would take to do so
>>> would make it hard. 
>> 
>> On the purely technical level, it may or may not be harder than mashing
>> everything into QEMU.
>> 
>> On the getting-patches-merged level, mashing everything into QEMU is a
>> non-starter, as Peter and I have told you multiple times.
>> 
>> That tips the balance somewhat.
>> 
>>>>> This patch can definitely be more simplified. QMP
>>>>> commands could be used in place of C functions. 
>>>>> This would reduce the patch size greatly. 
>>>> 
>>>> You're quite welcome to use QMP the way it wants to be used: as an
>>>> external interface.
>>>> 
>>>> Abusing it as internal interface won't fly.
>>> 
>>> The QMP interface is primarily there to help a gui interact with QEMU. That
>>> is what I intend to use it for.
>> 
>> Nope, the QMP interface's purpose is to let other programs interact with
>> QEMU.
>> 
>> You're free to use it for other purposes to your heart's content.  Just
>> don't count on patches to be merged when they do things maintainers have
>> told you not to do :)
>
> I did do as you said and used C functions in place of the original hmp
> commands.

Yes, you did, to address my hard objections there.  A hard objection is
about a technical issue in my area of expertise, and especially the
areas I maintain.  Unaddressed hard objections NAK patches.

I also have opinions on matters outside the areas I maintain, like
whether we should be in the GUI business, but mere opinions don't NAK
patches.

> I guess there never was any hope for this patch. :(

Getting a patch rejected isn't a pleasant experience.  Would you like to
see my collection of rejected patches?
Programmingkid Sept. 28, 2015, 8:11 p.m. UTC | #9
On Sep 28, 2015, at 4:09 PM, Markus Armbruster wrote:

> Programmingkid <programmingkidx@gmail.com> writes:
> 
>> On Sep 25, 2015, at 11:42 AM, Markus Armbruster wrote:
>> 
>>> Programmingkid <programmingkidx@gmail.com> writes:
>>> 
>>>> On Sep 24, 2015, at 2:57 AM, Markus Armbruster wrote:
>>>> 
>>>>> Programmingkid <programmingkidx@gmail.com> writes:
>>>>> 
>>>>>> On Sep 23, 2015, at 4:35 PM, Peter Maydell wrote:
>>>>>> 
>>>>>>> On 17 September 2015 at 21:17, Programmingkid
>>>>>>> <programmingkidx@gmail.com> wrote:
>>>>>>>> Add "Mount Image File..." and a "Eject Image File" menu items to
>>>>>>>> cocoa interface. This patch makes sharing files between the
>>>>>>>> host and the guest user-friendly.
>>>>>>>> 
>>>>>>>> The "Mount Image File..." menu item displays a dialog box having the
>>>>>>>> user pick an image file to use in QEMU. The image file is setup as
>>>>>>>> a USB flash drive. The user can do the equivalent of removing the
>>>>>>>> flash drive by selecting the file in the "Eject Image File" submenu.
>>>>>>>> 
>>>>>>>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>>>>>>> 
>>>>>>> I've thought a bit about this, and I really don't think this sort
>>>>>>> of feature should be part of QEMU itself. Our general design for
>>>>>>> how QEMU does this sort of thing is that an external program
>>>>>>> (virt-manager, for instance) is responsible for providing most
>>>>>>> of the UI conveniences the user wants, and QEMU's "ui" code is
>>>>>>> a fairly simple minimum-functionality affair. I agree with Markus
>>>>>>> that this separation of concerns has generally worked OK for us.
>>>>>>> 
>>>>>>> I don't think OSX should be an exception to this design model:
>>>>>>> (a) being an odd special case is never a good idea
>>>>>>> (b) as a practical matter, I'm the only person who really reviews
>>>>>>> OSX patches, and I don't have either the time nor the UI or OSX
>>>>>>> expertise to deal with maintaining what will effectively be a
>>>>>>> vm-manager grafted onto the side of QEMU
>>>>>>> 
>>>>>>> So I think your efforts would be better spent in either porting
>>>>>>> one of the Linux frontends like libvirt/virt-manager, or in
>>>>>>> writing a custom OSX specific frontend.
>>>>>> 
>>>>>> I understand that time is precious. It is one of those things
>>>>>> that we only have a finite amount of. Every user can agree
>>>>>> to that. This patch was pretty hairy looking with the QDict
>>>>>> and other unfamiliar code. With that said I'm not ready to
>>>>>> give up on this patch. It is a huge time saver for the user.
>>>>>> Without it, the user would need to spend a lot of time
>>>>>> investigating documentation. What's worse is the user
>>>>>> would have to type out full paths to files they need. This
>>>>>> would definitely be error prone and frustrating.
>>>>> 
>>>>> Nobody is challenging the idea that many users appreciate a GUI.
>>>>> 
>>>>> What we've been trying to tell you is where in this software layer cake
>>>>> the GUI should be.  In Peter's words, "our general design for how QEMU
>>>>> does this sort of thing is that an external program (virt-manager, for
>>>>> instance) is responsible for providing most of the UI conveniences".
>>>> 
>>>> That is easy for you to say. Linux already has virt-manager. Mac OS
>>>> X doesn't.
>>>> Expecting someone to just go and port another program to Mac OS X is 
>>>> unreasonable. The amount of time and energy it would take to do so
>>>> would make it hard. 
>>> 
>>> On the purely technical level, it may or may not be harder than mashing
>>> everything into QEMU.
>>> 
>>> On the getting-patches-merged level, mashing everything into QEMU is a
>>> non-starter, as Peter and I have told you multiple times.
>>> 
>>> That tips the balance somewhat.
>>> 
>>>>>> This patch can definitely be more simplified. QMP
>>>>>> commands could be used in place of C functions. 
>>>>>> This would reduce the patch size greatly. 
>>>>> 
>>>>> You're quite welcome to use QMP the way it wants to be used: as an
>>>>> external interface.
>>>>> 
>>>>> Abusing it as internal interface won't fly.
>>>> 
>>>> The QMP interface is primarily there to help a gui interact with QEMU. That
>>>> is what I intend to use it for.
>>> 
>>> Nope, the QMP interface's purpose is to let other programs interact with
>>> QEMU.
>>> 
>>> You're free to use it for other purposes to your heart's content.  Just
>>> don't count on patches to be merged when they do things maintainers have
>>> told you not to do :)
>> 
>> I did do as you said and used C functions in place of the original hmp
>> commands.
> 
> Yes, you did, to address my hard objections there.  A hard objection is
> about a technical issue in my area of expertise, and especially the
> areas I maintain.  Unaddressed hard objections NAK patches.
> 
> I also have opinions on matters outside the areas I maintain, like
> whether we should be in the GUI business, but mere opinions don't NAK
> patches.

Are you against the GTK interface that works on Linux?

> 
>> I guess there never was any hope for this patch. :(
> 
> Getting a patch rejected isn't a pleasant experience.  Would you like to
> see my collection of rejected patches?

No thank you.
diff mbox

Patch

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 334e6f6..49df138 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -31,6 +31,9 @@ 
 #include "sysemu/sysemu.h"
 #include "qmp-commands.h"
 #include "sysemu/blockdev.h"
+#include "include/monitor/qdev.h"
+#include "hw/boards.h"
+#include "include/hw/usb.h"
 
 #ifndef MAC_OS_X_VERSION_10_5
 #define MAC_OS_X_VERSION_10_5 1050
@@ -52,6 +55,8 @@ 
 #endif
 
 #define cgrect(nsrect) (*(CGRect *)&(nsrect))
+#define USB_DISK_ID "USB_DISK"
+#define EJECT_IMAGE_FILE_TAG 2099
 
 typedef struct {
     int width;
@@ -263,6 +268,28 @@  static void handleAnyDeviceErrors(Error * err)
     }
 }
 
+/* Determines if an image file is already being used by QEMU */
+static bool imageFileInUse(const char *file_name)
+{
+    BlockInfoList *current_device;
+
+    current_device = qmp_query_block(NULL);
+    while (current_device) {
+        if (!current_device->value || !current_device->value->inserted
+                                  || !current_device->value->inserted->file) {
+            current_device = current_device->next;
+            continue;
+        }
+
+        if(strcmp(current_device->value->inserted->file, file_name) == 0) {
+            return true; /* image file found */
+        }
+        current_device = current_device->next;
+    }
+
+    return false; /* image file not found */
+}
+
 /*
  ------------------------------------------------------
     QemuCocoaView
@@ -829,6 +856,9 @@  QemuCocoaView *cocoaView;
 - (void)powerDownQEMU:(id)sender;
 - (void)ejectDeviceMedia:(id)sender;
 - (void)changeDeviceMedia:(id)sender;
+- (void)mountImageFile:(id)sender;
+- (void)ejectImageFile:(id)sender;
+- (void)updateEjectImageMenuItems;
 @end
 
 @implementation QemuCocoaAppController
@@ -1125,6 +1155,192 @@  QemuCocoaView *cocoaView;
     }
 }
 
+/* Displays a dialog box asking the user for an image file to mount */
+- (void)mountImageFile:(id)sender
+{
+    /* Display the file open dialog */
+    NSOpenPanel * openPanel;
+    openPanel = [NSOpenPanel openPanel];
+    [openPanel setCanChooseFiles: YES];
+    [openPanel setAllowsMultipleSelection: NO];
+    [openPanel setAllowedFileTypes: supportedImageFileTypes];
+    if([openPanel runModal] == NSFileHandlingPanelOKButton) {
+
+        NSString * file = [[[openPanel URLs] objectAtIndex: 0] path];
+        static int usb_disk_count;
+        char *id_string, *file_name, *drive_options = NULL, *file_name_hint;
+        NSString *string_buffer;
+        const int file_name_hint_size = 10;
+        DriveInfo *dinfo;
+        QemuOpts *opts = NULL;
+        MachineClass *mc;
+        QObject *ret_data = NULL;
+        Error *errp = NULL;
+        QDict * qdict = NULL;
+
+        /* If the user didn't select a file */
+        if(file == nil) {
+            return;
+        }
+
+        /* If the file is already being used */
+        if(imageFileInUse([file cStringUsingEncoding: NSASCIIStringEncoding])) {
+            NSBeep();
+            QEMU_Alert(@"Error: file already in use.");
+            return;
+        }
+
+        string_buffer = [file lastPathComponent];
+        string_buffer = [string_buffer stringByDeletingPathExtension];
+        if([string_buffer length] > file_name_hint_size) {
+            string_buffer = [string_buffer substringToIndex:
+                                                           file_name_hint_size];
+        }
+        file_name_hint = g_strdup_printf("%s",
+                    [string_buffer cStringUsingEncoding:NSASCIIStringEncoding]);
+
+        id_string = g_strdup_printf("%s_%s_%d", USB_DISK_ID, file_name_hint,
+                                                              usb_disk_count++);
+
+        file_name = g_strdup_printf("%s",
+                            [file cStringUsingEncoding: NSASCIIStringEncoding]);
+
+        /* See if the file's name contains a comma */
+        if (strchr(file_name, ',')) {
+            NSBeep();
+            QEMU_Alert(@"Sorry but QEMU currently does not support files with"
+                        " commas in the file name.");
+            goto free_memory;
+        }
+
+        /* drive_add equivalent code */
+        drive_options = g_strdup_printf("if=none,id=%s,file=%s", id_string,
+                                                                     file_name);
+
+        opts = drive_def(drive_options);
+        if (!opts) {
+            QEMU_Alert(@"Error: could not create QemuOpts object!");
+            goto free_memory;
+        }
+
+        mc = MACHINE_GET_CLASS(current_machine);
+        dinfo = drive_new(opts, mc->block_default_type);
+        if (!dinfo) {
+            qemu_opts_del(opts);
+            QEMU_Alert(@"Error: could not create DriveInfo object!");
+            goto free_memory;
+        }
+
+        /* device_add equivalent code */
+
+        /* Create the QDict object */
+        qdict = qdict_new();
+        qdict_set_default_str(qdict, "driver", "usb-storage");
+        qdict_set_default_str(qdict, "id", id_string);
+        qdict_set_default_str(qdict, "drive", id_string);
+
+        qmp_device_add(qdict, &ret_data, &errp);
+        handleAnyDeviceErrors(errp);
+        [self updateEjectImageMenuItems];
+
+        free_memory:
+        g_free(qdict);
+        g_free(file_name);
+        g_free(file_name_hint);
+        g_free(id_string);
+        g_free(drive_options);
+        g_free(ret_data);
+    }
+}
+
+/* Removes an image file from QEMU */
+- (void)ejectImageFile:(id) sender
+{
+    NSString *image_file_ID;
+    Error *errp = NULL;
+
+    image_file_ID = [sender representedObject];
+    if (image_file_ID == nil) {
+        NSBeep();
+        QEMU_Alert(@"Could not find image file's ID!");
+        return;
+    }
+
+    qmp_device_del([image_file_ID cStringUsingEncoding: NSASCIIStringEncoding],
+                                                                         &errp);
+    handleAnyDeviceErrors(errp);
+    [self updateEjectImageMenuItems];
+}
+
+/* Gives each mounted image file an eject menu item */
+- (void) updateEjectImageMenuItems
+{
+    NSMenu *machine_menu;
+    NSMenu *eject_submenu;
+    BlockInfoList *current_device;
+    NSString *file_name, *device_name;
+    NSMenuItem *eject_file_menu_item;  /* Used with each mounted image file */
+
+    machine_menu = [[[NSApp mainMenu] itemWithTitle:@"Machine"] submenu];
+
+    /* Remove old menu items*/
+    eject_submenu = [[machine_menu itemWithTag: EJECT_IMAGE_FILE_TAG] submenu];
+    if(!eject_submenu) {
+        NSBeep();
+        QEMU_Alert(@"Failed to find eject submenu!");
+        return;
+    }
+    int index;
+    for (index = [eject_submenu numberOfItems]-1; index >= 0; index--) {
+        [eject_submenu removeItemAtIndex: index];
+    }
+
+    /* Look for mounted image files */
+    current_device = qmp_query_block(NULL);
+    while(current_device) {
+        if (!current_device->value || !current_device->value->inserted
+                                  || !current_device->value->inserted->file) {
+            current_device = current_device->next;
+            continue;
+        }
+
+        /* if the device's name is the generated ID */
+        if (!strstr(current_device->value->device, USB_DISK_ID)) {
+            current_device = current_device->next;
+            continue;
+        }
+
+        file_name = [NSString stringWithFormat: @"%s",
+                                         current_device->value->inserted->file];
+        file_name = [file_name lastPathComponent]; /* Get only the file name */
+
+        eject_file_menu_item = [[NSMenuItem alloc] initWithTitle:
+                             [NSString stringWithFormat: @"Eject %@", file_name]
+                                              action: @selector(ejectImageFile:)
+                                       keyEquivalent: @""];
+        [eject_submenu addItem: eject_file_menu_item];
+        device_name = [NSString stringWithFormat: @"%s",
+                                                 current_device->value->device];
+        [eject_file_menu_item setRepresentedObject: device_name];
+        [eject_file_menu_item autorelease];
+        current_device = current_device->next;
+    }
+
+    /* Add default menu item if submenu is empty */
+    if([eject_submenu numberOfItems] == 0) {
+
+        /* Create the default menu item */
+        NSMenuItem *empty_menu_item;
+        empty_menu_item = [NSMenuItem new];
+        [empty_menu_item setTitle: @"No items available"];
+        [empty_menu_item setEnabled: NO];
+
+        /* Add the default menu item to the submenu */
+        [eject_submenu addItem: empty_menu_item];
+        [empty_menu_item release];
+    }
+}
+
 @end
 
 
@@ -1338,10 +1554,39 @@  static void add_console_menu_entries(void)
     }
 }
 
+/* Adds menu items that can mount an image file like a usb flash drive */
+static void addImageMountingMenus(NSMenu *menu)
+{
+    [menu addItem: [[[NSMenuItem alloc] initWithTitle: @"Mount Image File..."
+           action: @selector(mountImageFile:) keyEquivalent: @""] autorelease]];
+
+    /* Create the eject menu item */
+    NSMenuItem *eject_menu_item;
+    eject_menu_item = [NSMenuItem new];
+    [eject_menu_item setTitle: @"Eject Image File"];
+    [eject_menu_item setTag: EJECT_IMAGE_FILE_TAG];
+    [menu addItem: eject_menu_item];
+    [eject_menu_item autorelease];
+
+    /* Create the default menu item for the eject menu item's submenu*/
+    NSMenuItem *empty_menu_item;
+    empty_menu_item = [NSMenuItem new];
+    [empty_menu_item setTitle: @"No items available"];
+    [empty_menu_item setEnabled: NO];
+    [empty_menu_item autorelease];
+
+    /* Add the default menu item to the submenu, the "No items" menu item */
+    NSMenu *submenu;
+    submenu = [NSMenu new];
+    [eject_menu_item setSubmenu: submenu];
+    [submenu addItem: empty_menu_item];
+    [submenu autorelease];
+}
+
 /* Make menu items for all removable devices.
  * Each device is given an 'Eject' and 'Change' menu item.
  */
-static void addRemovableDevicesMenuItems()
+static void addRemovableDevicesMenuItems(void)
 {
     NSMenu *menu;
     NSMenuItem *menuItem;
@@ -1402,6 +1647,11 @@  static void addRemovableDevicesMenuItems()
         currentDevice = currentDevice->next;
     }
     qapi_free_BlockInfoList(pointerToFree);
+
+    /* If USB is available use the image mounting feature */
+    if(usb_bus_find(-1)){
+        addImageMountingMenus(menu);
+    }
 }
 
 void cocoa_display_init(DisplayState *ds, int full_screen)