From mboxrd@z Thu Jan 1 00:00:00 1970 From: Markus Armbruster Subject: Re: [Qemu-devel] [PATCHv2 1/8] Introduce deriver_name field to DeviceInfo structure. Date: Sat, 06 Nov 2010 13:55:30 +0100 Message-ID: References: <1288525209-3303-2-git-send-email-gleb@redhat.com> <20101104094244.GC6018@redhat.com> <20101104154454.GB14910@redhat.com> <20101105154142.GA9617@redhat.com> <20101105183118.GC9617@redhat.com> <20101106115356.GF9617@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: blauwirbel@gmail.com, alex.williamson@redhat.com, qemu-devel@nongnu.org, kvm@vger.kernel.org To: Gleb Natapov Return-path: Received: from mx1.redhat.com ([209.132.183.28]:63351 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754716Ab0KFMze (ORCPT ); Sat, 6 Nov 2010 08:55:34 -0400 In-Reply-To: <20101106115356.GF9617@redhat.com> (Gleb Natapov's message of "Sat, 6 Nov 2010 13:53:56 +0200") Sender: kvm-owner@vger.kernel.org List-ID: Gleb Natapov writes: > On Sat, Nov 06, 2010 at 10:01:25AM +0100, Markus Armbruster wrote: > [skip] >> > Why should Seabios match against three (or even more) different type of >> > devices to detect ata interface? Why require Seabios changes when this >> > can be avoided (if new device that provide ata is added)? OpenBIOS also >> > supports qemu BTW (this is Open Firmware implementation for pc, you can >> > run and see what kind of device paths it generates). >> >> I think we've finally cut through the confusion caused by the >> unfortunate choice of driver_name for this new device attribute. >> >> The fact that you choose values of your driver_name in a way that is >> inspired by the syntactic conventions of IEEE 1275 is not its >> distinguishing characteristic. The values of existing member name are >> inspired by that as well. driver_name's distinguishing characteristic >> is its purpose: communication with SeaBIOS. >> > My understanding of this name in IEEE 1275 is that it specifies what > driver in FW handles a device. > >> I'm fine with you choosing its values however it's convenient for that >> purpose, as long as you give it a name reflecting that purpose. What >> about fw_name and qdev_fw_name()? >> > I am not attached to the name. Can "alias" be used for that purpose? alias needs to be an unambigous name of the device, just like name. If I understand you correctly, you want to use the same string for different, yet sufficiently compatible devices, so alias won't do. >> Next, I'm worried about overloading of method get_dev_path(). It's >> being used for a very specific purpose: savevm/loadvm. >> > This part of the patch is not completed yet. I intend to change the code > in savevm/loadvm to call qdev_get_dev_path() to get full device path > there. > >> * It's currently defined only for PCI devices. Your PATCH 7/8 changes >> its value there, from DOMAIN:BUS:SLOT.FUNCTION to FW_NAME@SLOT. >> > Old definition is buggy BTW. BUS part is controlled by a guest and may > be different from default value at destination. Yes, it's problematic for anything domain:bus 0:0. Which happens to be the only one that can occur here currently, as far as I know. >> - The old value identifies the qdev. The new value does not >> (remember, we have a separate qdev per PCI function). Why is this >> okay? >> > No no. New value is FW_NAME@SLOT,FUNC. Spec says that if FUNC is zero it > can be omitted. Okay. >> - Is the value saved with the VM? If yes, this is an incompatible >> change. > Don't understand that remark. If the value somehow makes it into the savevm data, then changing values could render the data incompatible: old qemu can't load new data, and vice versa. >> * You extend it for ISA and System bus (PATCH 5,6/8). How does this >> affect savevm? >> > We should ask savevm experts. As far as I can see it affects id > creation. As long as id is unique we should be OK. We may send more info > on migration after the patches. We definitely need review and testing there.