From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [Qemu-devel] [RFC PATCH 3/6] RAMBlock: Add a name field Date: Wed, 09 Jun 2010 08:57:44 -0500 Message-ID: <4C0F9DD8.5000400@codemonkey.ws> References: <20100608191447.4451.47795.stgit@localhost.localdomain> <201006090330.10324.paul@codesourcery.com> <4C0EFF39.9070602@codemonkey.ws> <201006090354.05197.paul@codesourcery.com> <4C0F81F2.4030106@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Paul Brook , qemu-devel@nongnu.org, chrisw@redhat.com, Alex Williamson , kvm@vger.kernel.org, quintela@redhat.com To: Avi Kivity Return-path: Received: from mail-gw0-f46.google.com ([74.125.83.46]:55458 "EHLO mail-gw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756423Ab0FIN5u (ORCPT ); Wed, 9 Jun 2010 09:57:50 -0400 Received: by gwj15 with SMTP id 15so41122gwj.19 for ; Wed, 09 Jun 2010 06:57:47 -0700 (PDT) In-Reply-To: <4C0F81F2.4030106@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 06/09/2010 06:58 AM, Avi Kivity wrote: > On 06/09/2010 05:54 AM, Paul Brook wrote: >>> On 06/08/2010 09:30 PM, Paul Brook wrote: >>>>> The offset given to a block created via qemu_ram_alloc/map() is >>>>> arbitrary, let the caller specify a name so we can make a positive >>>>> match. >>>>> >>>>> >>>>> @@ -1924,7 +1925,9 @@ static int pci_add_option_rom(PCIDevice *pdev) >>>>> + snprintf(name, sizeof(name), "pci:%02x.%x.rom", >>>>> + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn)); >>>>> + pdev->rom_offset = qemu_ram_alloc(name, size); >>>> This looks pretty bogus. It should be associated with the device, >>>> rather >>>> than incorrectly trying to generate a globally unique name. >>> Not all ram is associated with a device. >> Maybe not, but where it is we should be using that information. >> Absolute minimum we should be using the existing qdev address rather >> than >> inventing a new one. Duplicating this logic inside every device >> seems like a >> bad idea so I suggest identifying ram blocks by a (name, device) >> pair. For now >> we can allow a NULL device for system memory. >> > > I agree, this is duplicating the qdev tree in a string. Devices/BARs > should have ram qdev fields and so ram can be enumerated completely > via qdev. Keep in mind, this has to be a stable string across versions of qemu since this is savevm/migration. Are we absolutely confident that the full qdev path isn't going to change? I'm more confident that a unique device name is going to be static across qemu versions. Regards, Anthony Liguori > System memory can be part of a system device. >