- 
                Notifications
    You must be signed in to change notification settings 
- Fork 84
OpenXT build fixes #383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
          
     Open
      
      
            jandryuk
  wants to merge
  11
  commits into
  xapi-project:master
  
    
      
        
          
  
    
      Choose a base branch
      
     
    
      
        
      
      
        
          
          
        
        
          
            
              
              
              
  
           
        
        
          
            
              
              
           
        
       
     
  
        
          
            
          
            
          
        
       
    
      
from
jandryuk:build-fixes
  
      
      
   
  
    
  
  
  
 
  
      
    base: master
Could not load branches
            
              
  
    Branch not found: {{ refName }}
  
            
                
      Loading
              
            Could not load tags
            
            
              Nothing to show
            
              
  
            
                
      Loading
              
            Are you sure you want to change the base?
            Some commits from the old base branch may be removed from the timeline,
            and old review comments may become outdated.
          
          
                
     Open
            
            OpenXT build fixes #383
Conversation
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
    vhd_open_crypto has a redeclaration error:
block-crypto.c: In function ‘vhd_open_crypto’:
block-crypto.c:351:17: error: ‘key’ redeclared as different kind of symbol
  351 |         uint8_t key[MAX_AES_XTS_PLAIN_KEYSIZE / sizeof(uint8_t)] = { 0 };
      |                 ^~~
block-crypto.c:346:52: note: previous definition of ‘key’ with type ‘const uint8_t *’ {aka ‘const unsigned char *’}
  346 | vhd_open_crypto(vhd_context_t *vhd, const uint8_t *key, size_t key_bytes, const char *name)
OPEN_XT populates its key with chain_find_keyed_vhd, while the
non-OPEN_XT version has the key passed in.  Rename the local buffer and
assign key to it to keep things working.
chain_find_keyed_vhd can't accept the const key, so pass keybuf to it.
Signed-off-by: Jason Andryuk <[email protected]>
    GCC complains:
block-crypto.c: In function ‘find_keyfile’:
block-crypto.c:153:32: error: ‘,aes-xts-plain,’ directive output may be truncated writing 15 bytes into a region of size between 0 and 255 [-Werror=format-truncation=]
  153 |                          "%s/%s,aes-xts-plain,%d.key",
      |                                ^~~~~~~~~~~~~~~
block-crypto.c:152:17: note: ‘snprintf’ output 22 or more bytes (assuming 277) into a destination of size 256
  152 |                 snprintf(path, sizeof(path),
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
  153 |                          "%s/%s,aes-xts-plain,%d.key",
      |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  154 |                          keydir, basename, keysize);
      |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~
Increase the size of path.
Signed-off-by: Jason Andryuk <[email protected]>
    findkey() sets keysize to 512 or 256, but an xts case deals with bytes, better name the variables to indicate whether they contain keysize in bytes or bits, and make sure we convert chain_find_keyed_vhd to bytes before calling xts_aes_setkey() Signed-off-by: Chris Rogers <[email protected]> Signed-off-by: Jason Andryuk <[email protected]>
OpenXT doesn't pass in the encryption key, but loads it later when opening the VHD itself. Therefore it needs to always load the crypto library to have it available. Re-arrange to make that the case. While doing this, swap the if branches, to let the condition be a positive check. Signed-off-by: Jason Andryuk <[email protected]>
We need to include the util.h for the safe_strncpy declararion. Fixes: 6ffa1d8 "CA-366761: replace use of strncpy with inlined wrapper" Signed-off-by: Jason Andryuk <[email protected]>
              
                    MarkSymsCtx
  
              
              reviewed
              
                  
                    Nov 23, 2023 
                  
              
              
            
            
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor nit but other than it looks fine, apologies for the delay in reviewing it.
tapdisk_xenblkif_evtchn_event_id() tapdisk_xenblkif_chkrng_event_id() tapdisk_xenblkif_stoppolling_event_id() Can all be static inline functions within td-blkif.c Signed-off-by: Jason Andryuk <[email protected]>
Move into the header as a proper static inline. Signed-off-by: Jason Andryuk <[email protected]>
The __RD* macros are already defined in Xen's io/ring.h which is included through blkif.h and they have been there for years. In Xen 4.18 the macros changed for MISRA and now the definitions are out of sync leading to redefinition errors. Just remove the local versions and rely on the io/ring.h ones. Signed-off-by: Jason Andryuk <[email protected]>
In the lib/vhd code:
In function 'safe_strncpy',
    inlined from 'vhd_initialize_footer' at libvhd.c:2882:2:
include/util.h:46:17: error: 'strncpy' output truncated before terminating nul copying 3 bytes from a string of the same length [-Werror=stringop-truncation]
   46 |         pdest = strncpy(dest, src, n - 1);
In the lvm code twice:
In function 'safe_strncpy',
    inlined from 'lvm_copy_name' at lvm-util.c:71:2,
    inlined from 'lvm_scan_lvs' at lvm-util.c:289:9:
include/util.h:46:17: error: 'strncpy' output may be truncated copying 255 bytes from a string of length 255 [-Werror=stringop-truncation]
   46 |         pdest = strncpy(dest, src, n - 1);
For VHD, just set the footer bytes individually and avoid the string
ops.
For LVM, switching back to strncpy and providing the full destination
size silences the errors.  strncpy does NUL terminate the string - it
just may truncate.  (safe_strncpy silences many other instances of
stringop-truncation like:
"error: 'strncpy' specified bound 1024 equals destination size", so we
want to keep it.)
Signed-off-by: Jason Andryuk <[email protected]>
    OpenXT, using OpenEmbedded Dundell, fails to link cbt-util with errors like: ld: ./.libs/libcbtutil.a(cbt-util.o): in function `cbt_util_set': ld: cbt/cbt-util.c:360: undefined reference to `uuid_parse' libcbtutil.a/cbt-util.c reference the uuid funtions, and main.c doesn't have any references. Move the linking to the library to resolve the issue. Signed-off-by: Jason Andryuk <[email protected]>
fc8d8e0    to
    498a124      
    Compare
  
    
              
                    MarkSymsCtx
  
              
              previously approved these changes
              
                  
                    Nov 27, 2023 
                  
              
              
            
            
| I'll run this through some tests in our automation. | 
              
                    MarkSymsCtx
  
              
              approved these changes
              
                  
                    Mar 26, 2024 
                  
              
              
            
            
  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment
  
      
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
This is an assortment of build fixes from OpenXT, which is built with OpenEmbedded. Some are OpenXT specific and others are generic.
OPEN_XT needs a few fixups to build properly.
The __RD* macros throw a redefinition error with Xen 4.18.
Some fixes to build with -Werror=stringop-truncation.
Individual commits are documented.