[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 05/12] libxenguest: complete loops in xc_map_domain_meminfo()


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Mon, 28 Jun 2021 10:53:12 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=MNfBoWLeAchrpJjQXWjjbGgAEVvQCWKuiBky322Y9og=; b=bKHD/xMmy90uOl99i0Iq0ainZnk/3uyCIC8p4XD9mjbo1hCn9ErWVr5drQwAPJdcPgqifnFNwmJwxAbcHW3jZkXr9kcQB/UyullaB4BiQF7Sx+IHl4JFp7eCVkasSaDDlQp0uzSIkeu4VqEfO5xZ45SIcVvm4lPd6iJi+7tJwFPJojk6cNYjK9evbmcXD4YrluEHfBYPlSBPYqcXs5O0S2x1/+NdtxlBZHHyPwVAmLzrs6AbgkzQX+MkexVTF4tg6UMRFNu06aoSQequ88OvWNKqA8VAGDg8/5afXzX08oSSq1ExMVhxSc1NMakOHXdpuZCual2BjJCnolxJGAgkBg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ceCx7tw2LEPU1/d5cIwEqTG906wg4Qhi7/OLU6dL6ypL+7lrGTaEO5qQGt+GOel7v0ZlQeg0NyBjePQQgDFWf+sD7wN3XMBFKk+T/uiTDvDiOr8EsrAvoZH4rDswqhQ+IX+aHQLc0EvpAb7rak04dJhehnoH1Ga6Uh09nZArEcp2agT/GxOZbzEIqLaC5ud0LK6OWVCcqe3jk68KtldPZdTTBrEWsFpwUnM1+1/gc1SGgbIpGtyHt+kv3GQiP7GjFoLEy6+HZ6L3Fou/pf0DzDJZCGyx7UZOp7VS4SYD25CP0XVUKHrZ+JDvgKsK4dai5xdiY/FuXq4GmylaSh/+dw==
  • Authentication-results: lists.xenproject.org; dkim=none (message not signed) header.d=none;lists.xenproject.org; dmarc=none action=none header.from=suse.com;
  • Cc: Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Juergen Gross <jgross@xxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Mon, 28 Jun 2021 08:53:31 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 25.06.2021 20:30, Andrew Cooper wrote:
> On 25/06/2021 14:19, Jan Beulich wrote:
>> minfo->p2m_size may have more than 31 significant bits. Change the
>> induction variable to unsigned long, and (largely for signed-ness
>> consistency) a helper variable to unsigned int.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> --- a/tools/libs/guest/xg_domain.c
>> +++ b/tools/libs/guest/xg_domain.c
>> @@ -40,7 +40,7 @@ int xc_map_domain_meminfo(xc_interface *
>>      xc_dominfo_t info;
>>      shared_info_any_t *live_shinfo;
>>      xen_capabilities_info_t xen_caps = "";
>> -    int i;
>> +    unsigned long i;
>>  
>>      /* Only be initialized once */
>>      if ( minfo->pfn_type || minfo->p2m_table )
>> @@ -116,12 +116,12 @@ int xc_map_domain_meminfo(xc_interface *
>>      /* Retrieve PFN types in batches */
>>      for ( i = 0; i < minfo->p2m_size ; i+=1024 )
>>      {
>> -        int count = ((minfo->p2m_size - i ) > 1024 ) ?
>> -                        1024: (minfo->p2m_size - i);
>> +        unsigned int count = ((minfo->p2m_size - i) > 1024) ?
>> +                             1024 : (minfo->p2m_size - i);
> 
> min().

min() using 1024UL or MIN()? (I'll use the former unless you tell
me otherwise.)

> Otherwise, Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Thanks.

> This whole infrastructure is almost abandoned, and broken.  Its used by
> xen-mfndump (debugging only) and xen-hptool mem-offline.
> 
> The mem-offline functionally cannot possibly work usefully.  It is PV
> only, despite not having an HVM check, and in particular reads the dead
> page in an attempt to restore the contents elsewhere.  There is also no
> thought given to writes from outside sources, such as DMA from
> passthrough or a different dom0 foreign mapping.
> 
> This is perhaps ok as an academic demonstration of "can I shuffle memory
> behind an alive VM in ideal circumstances", but will be killed by the
> dom0 kernel if you ever try running it to resolve a real memory error on
> a VM, because there is no possibility of recovering the data.
> 
> The mem-offline functionality needs deleting.  It isn't production
> ready, and can't credibly be made so.

I definitely agree; I'm merely trying to address an anomaly found
while auditing the code for certain properties, without any claim
that afterwards any of this would really work.

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.