From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:60190) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QqglD-00088Z-P0 for qemu-devel@nongnu.org; Tue, 09 Aug 2011 03:30:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QqglC-0003w1-OH for qemu-devel@nongnu.org; Tue, 09 Aug 2011 03:29:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42171) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QqglC-0003vq-Bk for qemu-devel@nongnu.org; Tue, 09 Aug 2011 03:29:58 -0400 Message-ID: <4E40E2A0.9050405@redhat.com> Date: Tue, 09 Aug 2011 09:32:48 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1311179069-27882-1-git-send-email-armbru@redhat.com> <1311179069-27882-45-git-send-email-armbru@redhat.com> <4E3A5221.3030407@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 44/55] spitz tosa: Simplify "drive is suitable for microdrive" test List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: andrzej zaborowski Cc: Peter Maydell , quintela@redhat.com, dbaryshkov@gmail.com, stefano.stabellini@eu.citrix.com, qemu-devel@nongnu.org, Markus Armbruster , amit.shah@redhat.com, lcapitulino@redhat.com Am 09.08.2011 06:32, schrieb andrzej zaborowski: > On 4 August 2011 10:02, Kevin Wolf wrote: >> Am 03.08.2011 22:20, schrieb andrzej zaborowski: >>> On 3 August 2011 20:24, Markus Armbruster wrote: >>>> andrzej zaborowski writes: >>>>> On 3 August 2011 18:38, Markus Armbruster wrote: >>>>>> andrzej zaborowski writes: >>>>>>> 2. if the >>>>>>> underlaying storage can disappear for any other reason if that's >>>>>>> possible to check. >>>>>> >>>>>> bdrv_is_removable() *isn't* such a check. >>>>> >>>>> Obviously I wasn't claiming it is, just that it might be useful, but >>>>> not necessrily possible. After all pretty much any storage can be >>>>> "ejected" with enough force, depending on how far you want to go. >>>>> >>>>>>>> What's wrong with that again? All sounds sensible to me. >>>>>>> >>>>>>> I'm not claiming otherwise, just double-checking this is what you want. >>>>> >>>>> So first you said you had a problem with _is_removable, and then you >>>>> said nothing was wrong with the implementation you outlined, plase >>>>> make up your mind. >>>> >>>> I don't appreciate you quoting me out of context like that. >>> >>> I got quite annoyed when you started putting words in my mouth by >>> saying I said anything about CD-ROM.. the code in spitz/tosa is not >>> concerned with CD-ROMs even if downstream it boils down to that, it is >>> concerned with whether the device is removable or not, and that's what >>> the check does. It doesn't help readability or anything by inlining >>> that check. If you're trying to check for A then don't spell it out >>> as B, be explicit. It's not a big deal but I just don't see the >>> point, sorry. >>> >>>> >>>> The sentence you quoted was in the middle of my attempt to get you to >>>> explain what you're trying to accomplish there. >>> >>> I already said about 3 times what it's trying to acomplish. You also >>> have used the word "removable" so I'm sure you know what it means and >>> don't need further explanation. But let's define it this way: if a >>> GUI is going to display an "eject" button next to a drive in the qemu >>> device tree, that's a removable device. CD-ROM is an example of that. >>> An IDE HDD is an example of something that's not going to have that >>> button (I assume). >> >> But this is a property of the device, not of the backend. This means >> that it belongs in the device emulation and not in block.c. > > By device do you mean hw/ide/microdrive.c? I mean just anything in hw/. I'm not familiar with how these devices work internally, so if hw/ide/microdrive.c is the right place, fine. > I'm not saying it belongs > in block.c, but logically it belongs in the same place as > bdrv_is_inserted, bdrv_is_locked, bdrv_eject etc. no? So it is a > property of whatever "media" is property of. Depends. As long as you're talking about purely virtual device state, I would say yes, they belong there, too. However, we have things like CD-ROM passthrough where bdrv_is_inserted etc. are supposed to return the status of the physical host device. These are host state and need to be in the block layer. Kevin